emit .att_syntax when global/naked asm use that option
#143599
Conversation
rustbot
added
A-LLVM
folkertdev
force-pushed
the
x86-asm-syntax-global-naked-asm
branch
from
2d97a2b to
8294047
Compare
| @@ -431,7 +435,11 @@ impl<'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { | |||
| } | |||
| } | |||
| } | |||
| if intel_syntax { | |||
|
|
|||
| // Just to play it safe, reset the assembly syntax to att. | |||
There was a problem hiding this comment.
Wouldn't this cause issues when -Cllvm-args=-x86-asm-syntax=intel is used? LLVM might still be trying to emit intel syntax asm for functions it compiles.
There was a problem hiding this comment.
LLVM doesn't emit asm as text, it emits the instructions directly and ignores the syntax. This only potentially affects inline asm in C code which would probably break anyways because most C code expects AT&T syntax for inline asm.
There was a problem hiding this comment.
Well it works in the test? I believe what is really happening is that every asm block is isolated, given that LLVM parses the assembly anyway.
There was a problem hiding this comment.
LLVM doesn't emit asm as text, it emits the instructions directly and ignores the syntax.
I think it would also break --emit asm producing valid assembly. Or does LLVM parse the asm blocks and then emit them as strings in the other format again?
Well it works in the test? I believe what is really happening is that every asm block is isolated, given that LLVM parses the assembly anyway.
LLVM definitively merges global asm blocks (unfortunately). For example assembler macros leak between global asm blocks.
There was a problem hiding this comment.
The parsing at least seems to happen independently? Anyway this was already what we emitted, and the problem that got reported is that .att_syntax just got ignored, which this PR fixes.
| if intel_syntax { | ||
| template_str.push_str(".intel_syntax\n"); | ||
|
|
||
| // Select the assembly syntax. On non-X86 platforms att is used by default, on X86 we use |
There was a problem hiding this comment.
Slight correction to the comment: the name AT&T refers specifically to an x86 asm syntax. There is no such thing as AT&T syntax on non-x86 platforms.
There was a problem hiding this comment.
Huh, I've always thought of it as intel and "the other one". At least s390x, powerpc, etc seem more inspired by AT&T.
Anyway, I rephrased that comment.
There was a problem hiding this comment.
Intel syntax is closer to the standard asm syntax on most architectures with the destination register coming first. AT&T syntax is actually based on PDP-11 assembly since that was the original platform for UNIX and the x86 port adapted the existing assembler instead of using the standard syntax.
folkertdev
force-pushed
the
x86-asm-syntax-global-naked-asm
branch
from
8294047 to
dcdb451
Compare
folkertdev
force-pushed
the
x86-asm-syntax-global-naked-asm
branch
from
dcdb451 to
6fc5d4e
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
Rollup of 9 pull requests Successful merges: - #143446 (use `--dynamic-list` for exporting executable symbols) - #143590 (Fix weird rustdoc output when single and glob reexport conflict on a name) - #143599 (emit `.att_syntax` when global/naked asm use that option) - #143615 (Fix handling of no_std targets in `doc::Std` step) - #143632 (fix: correct parameter names in LLVMRustBuildMinNum and LLVMRustBuildMaxNum FFI declarations) - #143640 (Constify `Fn*` traits) - #143651 (Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic) - #143660 (Disable docs for `compiler-builtins` and `sysroot`) - #143665 ([rustdoc-json] Add tests for `#[doc(hidden)]` handling of items.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143599 - folkertdev:x86-asm-syntax-global-naked-asm, r=Amanieu emit `.att_syntax` when global/naked asm use that option fixes #143542 LLVM would error when using `-Cllvm-args=-x86-asm-syntax=intel` in combination with global/naked assembly with `att_syntax`. It turns out that for LLVM you do in this case need to emit `.att_syntax`. r? `@Amanieu`
Rollup of 9 pull requests Successful merges: - rust-lang/rust#143446 (use `--dynamic-list` for exporting executable symbols) - rust-lang/rust#143590 (Fix weird rustdoc output when single and glob reexport conflict on a name) - rust-lang/rust#143599 (emit `.att_syntax` when global/naked asm use that option) - rust-lang/rust#143615 (Fix handling of no_std targets in `doc::Std` step) - rust-lang/rust#143632 (fix: correct parameter names in LLVMRustBuildMinNum and LLVMRustBuildMaxNum FFI declarations) - rust-lang/rust#143640 (Constify `Fn*` traits) - rust-lang/rust#143651 (Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic) - rust-lang/rust#143660 (Disable docs for `compiler-builtins` and `sysroot`) - rust-lang/rust#143665 ([rustdoc-json] Add tests for `#[doc(hidden)]` handling of items.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - rust-lang/rust#143446 (use `--dynamic-list` for exporting executable symbols) - rust-lang/rust#143590 (Fix weird rustdoc output when single and glob reexport conflict on a name) - rust-lang/rust#143599 (emit `.att_syntax` when global/naked asm use that option) - rust-lang/rust#143615 (Fix handling of no_std targets in `doc::Std` step) - rust-lang/rust#143632 (fix: correct parameter names in LLVMRustBuildMinNum and LLVMRustBuildMaxNum FFI declarations) - rust-lang/rust#143640 (Constify `Fn*` traits) - rust-lang/rust#143651 (Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic) - rust-lang/rust#143660 (Disable docs for `compiler-builtins` and `sysroot`) - rust-lang/rust#143665 ([rustdoc-json] Add tests for `#[doc(hidden)]` handling of items.) r? `@ghost` `@rustbot` modify labels: rollup
fixes #143542
LLVM would error when using
-Cllvm-args=-x86-asm-syntax=intelin combination with global/naked assembly withatt_syntax. It turns out that for LLVM you do in this case need to emit.att_syntax.r? @Amanieu