-
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
Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly #130630
Conversation
…0x inline assembly
Some changes occurred in compiler/rustc_codegen_gcc |
Thanks for working on SystemZ support for this!
As I read the Rust inline-asm documention (https://doc.rust-lang.org/stable/reference/inline-assembly.html), this looks OK to me. We have the statement:
Since the flags register cannot be specified as an input or output, the target-specific rules exception applies. For the flags register specifically, we then have the following rules:
We can define this for SystemZ, and the definition that makes sense is to preserve the full CC value. (And I guess also some of the flags in the floating-point control register, specifically the exception masks and rounding modes. But since we don't really model the FPC, we may have to just ignore that part for now.) Finally, looking at the list of clobbered registers on the other supported targets shows that for none of them,
Yes, that should be fine to ignore.
Not sure exactly what the Rust convention here is. I'd be fine with "areg", but maybe "acreg" or something might be clearer?
Interesting, thanks for pointing this out. This should be fixed in GCC at some point, but it's probably not particularly important. I agree it should not block this PR.
Actually, the situation has improved: the problem I pointed out in that last comment has been fixed in LLVM 16, which now always uses the same data_layout string, no matter whether the vector feature is enabled or not: Assuming rustc is now always built with LLVM 16 or later, we can drop the code that forces the vector feature to be always off, which would enable making general use of vector facilities if enabled. This would include fully supporting the vreg class for inline asm as well as enabling auto-SIMD and possibly even defining a set of platform-specific vector intrinsics. |
Thanks for looking into this! After reading your comment I agree that this is okay as is (clobber_abi` doesn't t affect the flags registers).
Thanks for confirming!
If there is a name familiar to people familiar with s390x, I would have preferred to use it, but if there is not,
Thanks for the info! The current minimum external LLVM version is 18 (#130487), so I thought that the explicit disabling of the vector feature could be removed, but reading the target spec code, it seems that there is actually another issue that needs to be resolved...
(I think it is off topic to continue talking about this here, so I opened #130869 which tracks the status of vector facilities support.) |
Well, what should be familiar is the term "access register" as well as the short form AR, which parallels the short forms GPR (general-purpose register), FPR (floating-point register), VR (vector register) and CR (control register). There is no generally-known term "areg", but then there is no generally-known term "freg" or "vreg" either. But given this way of forming the name, it should be clear what is meant by "areg". In any case, ARs by any name are basically never used on Linux anyway - sort of similar to segment registers on 64-bit Intel Linux. (With the exception of the toolchain using %a0/%a1 as thread-local pointer.)
Thanks! Will reply there. |
I'm not too concerned about the @bors r+ |
…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#130630 (Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly) - rust-lang#131042 (Instantiate binders in `supertrait_vtable_slot`) - rust-lang#131079 (Update wasm-component-ld to 0.5.9) - rust-lang#131085 (make test_lots_of_insertions test take less long in Miri) - rust-lang#131088 (add fixme to remove LLVM_ENABLE_TERMINFO when minimal llvm version is 19) r? `@ghost` `@rustbot` modify labels: rollup
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 supports
clobber_abi
which is one of the requirements of stabilization mentioned in #93335.This also supports vector registers (as
vreg
) and access registers (asareg
) as clobber-only, which need to support clobbering of them to implement clobber_abi.Refs:
I have three questions:
ELF Application Binary Interface s390x Supplement says thatUPDATE: resolved Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly #130630 (comment)cc
(condition code, bits 18-19 of PSW) is "Volatile".However, we do not have a register class for
cc
and instead markcc
as clobbered unlesspreserves_flags
is specified (Mark s390x condition code register as clobbered in inline assembly #111331).Therefore, in the current implementation, if both
preserves_flags
andclobber_abi
are specified,cc
is not marked as clobbered. Is this okay? Or even ifpreserves_flags
is used, shouldcc
be marked as clobbered ifclobber_abi
is used?ELF Application Binary Interface s390x Supplement says thatUPDATE: resolved Support clobber_abi and vector/access registers (clobber-only) in s390x inline assembly #130630 (comment)pm
(program mask, bits 20-23 of PSW) is "Cleared".There does not appear to be any registers associated with this in either LLVM or GCC, so at this point I don't see any way other than to just ignore it. Is this okay as-is?
reg_addr
, which uses the “a” constraint (Support reg_addr register class in s390x inline assembly #119431)...Note:
a0
anda1
, and usinga[2-15]
causes errors.Given that cg_gcc has a similar problem with other architecture (x86_64 asm: Build error when using r[8-15]b 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 thevector
target feature added in rustc_target: add known safe s390x target features #127506 is enabled, but core_arch has no s390x vector type and both#[repr(simd)]
andcore::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... S390x inline asm #88245 (comment)cc @uweigand
r? @Amanieu
@rustbot label +O-SystemZ +A-inline-assembly