-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
S390x inline asm #88245
S390x inline asm #88245
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon. Please see the contribution instructions for more information. |
r? rust-lang/wg-llvm |
r? @nagisa |
r? @Amanieu |
// CHECK: #APP | ||
// CHECK: ldr %f{{[0-9]+}}, %f{{[0-9]+}} | ||
// CHECK: #NO_APP | ||
check!(reg_f64, f64, freg, "ldr"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests look good, but you also need tests which use explicit registers like r0
and f0
.
// CHECK: #APP | ||
// CHECK: lgr %r{{[0-9]+}}, %r{{[0-9]+}} | ||
// CHECK: #NO_APP | ||
check!(reg_i64, i64, reg, "lgr"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test with pointer types. Refer to the other tests for examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the test I have for the pointer type, similar to reg_ptr
in x86-types.rs
:
// CHECK-LABEL: reg_ptr:
// CHECK: #APP
// CHECK: lgr %r{{[0-9]+}}, %r{{[0-9]+}}
// CHECK: #NO_APP
check!(reg_ptr, ptr, reg, "lgr");
and stderr
:
/home/linux1/rust/src/test/assembly/asm/s390x-types.rs:128:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: reg_ptr:
^
/home/linux1/rust/build/s390x-unknown-linux-gnu/test/assembly/asm/s390x-types.s390x/s390x-types.s:174:9: note: scanning from here
reg_f64:
^
/home/linux1/rust/build/s390x-unknown-linux-gnu/test/assembly/asm/s390x-types.s390x/s390x-types.s:204:9: note: possible intended match here
.globl reg_ptr
I am not sure why reg_ptr
is .globl
, nor how to go about resolving this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you paste the full contents of /home/linux1/rust/build/s390x-unknown-linux-gnu/test/assembly/asm/s390x-types.s390x/s390x-types.s
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.text
.file "s390x_types.63e18542-cgu.0"
.section .text.sym_fn_32,"ax",@progbits
.globl sym_fn_32
.p2align 4
.type sym_fn_32,@function
sym_fn_32:
.cfi_startproc
stmg %r14, %r15, 112(%r15)
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
#APP
brasl %r14, extern_func
#NO_APP
lmg %r14, %r15, 272(%r15)
br %r14
.Lfunc_end0:
.size sym_fn_32, .Lfunc_end0-sym_fn_32
.cfi_endproc
.section .text.sym_static,"ax",@progbits
.globl sym_static
.p2align 4
.type sym_static,@function
sym_static:
.cfi_startproc
stmg %r14, %r15, 112(%r15)
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
#APP
brasl %r14, extern_static
#NO_APP
lmg %r14, %r15, 272(%r15)
br %r14
.Lfunc_end1:
.size sym_static, .Lfunc_end1-sym_static
.cfi_endproc
.section .text.reg_i8,"ax",@progbits
.globl reg_i8
.p2align 4
.type reg_i8,@function
reg_i8:
.cfi_startproc
stmg %r13, %r15, 104(%r15)
.cfi_offset %r13, -56
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
lr %r13, %r2
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
lgr %r2, %r13
#NO_APP
lmg %r13, %r15, 264(%r15)
br %r14
.Lfunc_end2:
.size reg_i8, .Lfunc_end2-reg_i8
.cfi_endproc
.section .text.reg_i16,"ax",@progbits
.globl reg_i16
.p2align 4
.type reg_i16,@function
reg_i16:
.cfi_startproc
stmg %r13, %r15, 104(%r15)
.cfi_offset %r13, -56
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
lr %r13, %r2
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
lgr %r2, %r13
#NO_APP
lmg %r13, %r15, 264(%r15)
br %r14
.Lfunc_end3:
.size reg_i16, .Lfunc_end3-reg_i16
.cfi_endproc
.section .text.reg_i32,"ax",@progbits
.globl reg_i32
.p2align 4
.type reg_i32,@function
reg_i32:
.cfi_startproc
stmg %r13, %r15, 104(%r15)
.cfi_offset %r13, -56
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
lr %r13, %r2
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
lgr %r2, %r13
#NO_APP
lmg %r13, %r15, 264(%r15)
br %r14
.Lfunc_end4:
.size reg_i32, .Lfunc_end4-reg_i32
.cfi_endproc
.section .text.reg_i64,"ax",@progbits
.globl reg_i64
.p2align 4
.type reg_i64,@function
reg_i64:
.cfi_startproc
stmg %r13, %r15, 104(%r15)
.cfi_offset %r13, -56
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -160
.cfi_def_cfa_offset 320
lgr %r13, %r2
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
lgr %r2, %r13
#NO_APP
lmg %r13, %r15, 264(%r15)
br %r14
.Lfunc_end5:
.size reg_i64, .Lfunc_end5-reg_i64
.cfi_endproc
.section .text.reg_f32,"ax",@progbits
.globl reg_f32
.p2align 4
.type reg_f32,@function
reg_f32:
.cfi_startproc
stmg %r14, %r15, 112(%r15)
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -168
.cfi_def_cfa_offset 328
std %f8, 160(%r15)
.cfi_offset %f8, -168
ler %f8, %f0
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
ler %f0, %f8
#NO_APP
ld %f8, 160(%r15)
lmg %r14, %r15, 280(%r15)
br %r14
.Lfunc_end6:
.size reg_f32, .Lfunc_end6-reg_f32
.cfi_endproc
.section .text.reg_f64,"ax",@progbits
.globl reg_f64
.p2align 4
.type reg_f64,@function
reg_f64:
.cfi_startproc
stmg %r14, %r15, 112(%r15)
.cfi_offset %r14, -48
.cfi_offset %r15, -40
aghi %r15, -168
.cfi_def_cfa_offset 328
std %f8, 160(%r15)
.cfi_offset %f8, -168
ldr %f8, %f0
larl %r2, .L__unnamed_1
lghi %r3, 4
brasl %r14, dont_merge@PLT
#APP
ldr %f0, %f8
#NO_APP
ld %f8, 160(%r15)
lmg %r14, %r15, 280(%r15)
br %r14
.Lfunc_end7:
.size reg_f64, .Lfunc_end7-reg_f64
.cfi_endproc
.type .L__unnamed_1,@object
.section .rodata.cst4,"aM",@progbits,4
.p2align 1
.L__unnamed_1:
.ascii "func"
.size .L__unnamed_1, 4
.globl reg_ptr
.type reg_ptr,@function
.set reg_ptr, reg_i64
.section ".note.GNU-stack","",@progbits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one general comment I have is that there seems to be no support at all for vector registers in the patch right now.
The vector feature is turned off by default in |
…rrors; added register prefix
Ah, it's the data_layout problem again. That's unfortunate, but not trivial to fix. So I'm fine with not supporting vector registers either for now. |
Is there anywhere I can read more about this problem? |
@bors r+ |
📌 Commit 4a9ba65 has been approved by |
On s390x, presence of the vector feature changes the default alignment of the v128 type. This unfortunately means that the data_layout string is not constant for the architecture, but depends on whether or not the feature is enabled. The Rust compiler assumes that for each architecture, there is just a single constant data_layout string, which is true everywhere else but not on s390x. As long as this is the case, Rust has to decide whether to support the version of s390x with or without the vector feature, and currently the choice is to support the version without. |
S390x inline asm This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests. Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
S390x inline asm This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests. Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
S390x inline asm This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests. Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
S390x inline asm This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing rust-lang#85931; as well as a few tests. Further testing is needed, but I am a little unsure of what specific tests should be added to `src/test/assembly/asm/s390x.rs` to address this.
⌛ Testing commit 4a9ba65 with merge 6b92ac4559297fc34f846e4846eea72a501f1dfc... |
@bors retry |
☀️ Test successful - checks-actions |
Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly This supports `clobber_abi` which is one of the requirements of stabilization mentioned in rust-lang#93335. This also supports vector registers (as `vreg`) and access registers (as `areg`) as clobber-only, which need to support clobbering of them to implement clobber_abi. Refs: - "1.2.1.1. Register Preservation Rules" section in ELF Application Binary Interface s390x Supplement, Version 1.6.1 (lzsabi_s390x.pdf in https://github.com/IBM/s390x-abi/releases/tag/v1.6.1) - Register definition in LLVM: - Vector registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L249 - Access registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L332 I have three questions: - ~~ELF Application Binary Interface s390x Supplement says that `cc` (condition code, bits 18-19 of PSW) is "Volatile". However, we do not have a register class for `cc` and instead mark `cc` as clobbered unless `preserves_flags` is specified (rust-lang#111331). Therefore, in the current implementation, if both `preserves_flags` and `clobber_abi` are specified, `cc` is not marked as clobbered. Is this okay? Or even if `preserves_flags` is used, should `cc` be marked as clobbered if `clobber_abi` is used?~~ UPDATE: resolved rust-lang#130630 (comment) - ~~ELF Application Binary Interface s390x Supplement says that `pm` (program mask, bits 20-23 of PSW) is "Cleared". There does not appear to be any registers associated with this in either [LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td) or [GCC](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L407-L431), so at this point I don't see any way other than to just ignore it. Is this okay as-is?~~ UPDATE: resolved rust-lang#130630 (comment) - Is "areg" a good name for register class name for access registers? It may be a bit confusing between that and `reg_addr`, which uses the “a” constraint (rust-lang#119431)... Note: - GCC seems to [recognize only `a0` and `a1`](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L428-L429), and using `a[2-15]` [causes errors](https://godbolt.org/z/a46vx8jjn). Given that cg_gcc has a similar problem with other architecture (rust-lang/rustc_codegen_gcc#485), I don't feel this is a blocker for this PR, but it is worth mentioning here. - `vreg` should be able to accept `#[repr(simd)]` types as input if the `vector` target feature added in rust-lang#127506 is enabled, but core_arch has no s390x vector type and both `#[repr(simd)]` and `core::simd` are unstable, so I have not implemented it in this PR. EDIT: And supporting it is probably more complex than doing the equivalent on other architectures... rust-lang#88245 (comment) cc `@uweigand` r? `@Amanieu` `@rustbot` label +O-SystemZ
Rollup merge of rust-lang#130630 - taiki-e:s390x-clobber-abi, r=Amanieu Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly This supports `clobber_abi` which is one of the requirements of stabilization mentioned in rust-lang#93335. This also supports vector registers (as `vreg`) and access registers (as `areg`) as clobber-only, which need to support clobbering of them to implement clobber_abi. Refs: - "1.2.1.1. Register Preservation Rules" section in ELF Application Binary Interface s390x Supplement, Version 1.6.1 (lzsabi_s390x.pdf in https://github.com/IBM/s390x-abi/releases/tag/v1.6.1) - Register definition in LLVM: - Vector registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L249 - Access registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L332 I have three questions: - ~~ELF Application Binary Interface s390x Supplement says that `cc` (condition code, bits 18-19 of PSW) is "Volatile". However, we do not have a register class for `cc` and instead mark `cc` as clobbered unless `preserves_flags` is specified (rust-lang#111331). Therefore, in the current implementation, if both `preserves_flags` and `clobber_abi` are specified, `cc` is not marked as clobbered. Is this okay? Or even if `preserves_flags` is used, should `cc` be marked as clobbered if `clobber_abi` is used?~~ UPDATE: resolved rust-lang#130630 (comment) - ~~ELF Application Binary Interface s390x Supplement says that `pm` (program mask, bits 20-23 of PSW) is "Cleared". There does not appear to be any registers associated with this in either [LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td) or [GCC](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L407-L431), so at this point I don't see any way other than to just ignore it. Is this okay as-is?~~ UPDATE: resolved rust-lang#130630 (comment) - Is "areg" a good name for register class name for access registers? It may be a bit confusing between that and `reg_addr`, which uses the “a” constraint (rust-lang#119431)... Note: - GCC seems to [recognize only `a0` and `a1`](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L428-L429), and using `a[2-15]` [causes errors](https://godbolt.org/z/a46vx8jjn). Given that cg_gcc has a similar problem with other architecture (rust-lang/rustc_codegen_gcc#485), I don't feel this is a blocker for this PR, but it is worth mentioning here. - `vreg` should be able to accept `#[repr(simd)]` types as input if the `vector` target feature added in rust-lang#127506 is enabled, but core_arch has no s390x vector type and both `#[repr(simd)]` and `core::simd` are unstable, so I have not implemented it in this PR. EDIT: And supporting it is probably more complex than doing the equivalent on other architectures... rust-lang#88245 (comment) cc `@uweigand` r? `@Amanieu` `@rustbot` label +O-SystemZ
Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly This supports `clobber_abi` which is one of the requirements of stabilization mentioned in #93335. This also supports vector registers (as `vreg`) and access registers (as `areg`) as clobber-only, which need to support clobbering of them to implement clobber_abi. Refs: - "1.2.1.1. Register Preservation Rules" section in ELF Application Binary Interface s390x Supplement, Version 1.6.1 (lzsabi_s390x.pdf in https://github.com/IBM/s390x-abi/releases/tag/v1.6.1) - Register definition in LLVM: - Vector registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L249 - Access registers https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td#L332 I have three questions: - ~~ELF Application Binary Interface s390x Supplement says that `cc` (condition code, bits 18-19 of PSW) is "Volatile". However, we do not have a register class for `cc` and instead mark `cc` as clobbered unless `preserves_flags` is specified (rust-lang/rust#111331). Therefore, in the current implementation, if both `preserves_flags` and `clobber_abi` are specified, `cc` is not marked as clobbered. Is this okay? Or even if `preserves_flags` is used, should `cc` be marked as clobbered if `clobber_abi` is used?~~ UPDATE: resolved rust-lang/rust#130630 (comment) - ~~ELF Application Binary Interface s390x Supplement says that `pm` (program mask, bits 20-23 of PSW) is "Cleared". There does not appear to be any registers associated with this in either [LLVM](https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td) or [GCC](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L407-L431), so at this point I don't see any way other than to just ignore it. Is this okay as-is?~~ UPDATE: resolved rust-lang/rust#130630 (comment) - Is "areg" a good name for register class name for access registers? It may be a bit confusing between that and `reg_addr`, which uses the “a” constraint (rust-lang/rust#119431)... Note: - GCC seems to [recognize only `a0` and `a1`](https://github.com/gcc-mirror/gcc/blob/33ccc1314dcdb0b988a9276ca6b6ce9b07bea21e/gcc/config/s390/s390.h#L428-L429), and using `a[2-15]` [causes errors](https://godbolt.org/z/a46vx8jjn). Given that cg_gcc has a similar problem with other architecture (#485), I don't feel this is a blocker for this PR, but it is worth mentioning here. - `vreg` should be able to accept `#[repr(simd)]` types as input if the `vector` target feature added in rust-lang/rust#127506 is enabled, but core_arch has no s390x vector type and both `#[repr(simd)]` and `core::simd` are unstable, so I have not implemented it in this PR. EDIT: And supporting it is probably more complex than doing the equivalent on other architectures... rust-lang/rust#88245 (comment) cc `@uweigand` r? `@Amanieu` `@rustbot` label +O-SystemZ
This adds register definitions and constraint codes for the s390x general and floating point registers necessary for fixing #85931; as well as a few tests.
Further testing is needed, but I am a little unsure of what specific tests should be added to
src/test/assembly/asm/s390x.rs
to address this.