Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not handle inline assembly with "intel" flag as AT&T syntax #14264

Merged
merged 4 commits into from
Jan 29, 2024

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jan 28, 2024

LLVM understands this parameter and Crystal's parser also supports the "intel" attribute for Asm nodes, yet actually using the Intel syntax in the template string will produce a codegen error. This PR fixes that, and the emitted LLVM IR will now include the inteldialect option, e.g. call void asm inteldialect "mov dword ptr [$0], 42", "r"(ptr %x).

LLVM's own inline assembler should understand Intel syntax as long as it targets x86. This has nothing to do with whether --emit=asm writes the compiled assembly back using Intel syntax.

@HertzDevil HertzDevil changed the title Support inline x86 assembly using Intel syntax Do not handle inline assembly with "intel" flag as AT&T syntax Jan 28, 2024
src/llvm/type.cr Outdated Show resolved Hide resolved
HertzDevil and others added 2 commits January 28, 2024 20:18
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
@straight-shoota
Copy link
Member

suggestion: It might be more future proof for the method parameter to receive an instance of LibLLVM::InlineAsmDialect. Then intel_dialect: true would be come dialect: :intel and if there's ever another dialect added, it would work the same way.

@straight-shoota straight-shoota added this to the 1.12.0 milestone Jan 28, 2024
@straight-shoota straight-shoota merged commit 4b8f5c5 into crystal-lang:master Jan 29, 2024
57 checks passed
@HertzDevil HertzDevil deleted the bug/asm-intel branch January 30, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants