asm: Add a kreg0 register class on x86 which includes k0 #95740
Conversation
rustbot
added
the
T-compiler
|
Some changes occured to rustc_codegen_gcc cc @antoyo |
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
Amanieu
force-pushed
the
kreg0
branch
2 times, most recently
from
260e932
to
d152363
Compare
|
This looks correct to me but I'd like someone more familiar with this space to double check. r? @nagisa perhaps? |
| @@ -29,8 +29,6 @@ fn main() { | |||
| //~^ ERROR invalid register `rsp`: the stack pointer cannot be used as an operand | |||
| asm!("", in("ip") foo); | |||
| //~^ ERROR invalid register `ip`: the instruction pointer cannot be used as an operand | |||
| asm!("", in("k0") foo); | |||
There was a problem hiding this comment.
Why is this being removed? It is still invalid to use k0 as an input/output operand, right?
There was a problem hiding this comment.
No, it's now valid. I previously incorrectly thought k0 wasn't a real register, like xzr on AArch64.
There was a problem hiding this comment.
I think in that case the description should be updated
However k0 is a valid register and we need to have a way of marking it as clobbered for clobber_abi.
somewhat misleadingly led me to believe that k0 was only being exposed for use as a clobber.
compiler/rustc_target/src/asm/x86.rs
Outdated
| k0: kreg0 = ["k0"], | ||
| k1: kreg, kreg0 = ["k1"], | ||
| k2: kreg, kreg0 = ["k2"], | ||
| k3: kreg, kreg0 = ["k3"], | ||
| k4: kreg, kreg0 = ["k4"], | ||
| k5: kreg, kreg0 = ["k5"], | ||
| k6: kreg, kreg0 = ["k6"], | ||
| k7: kreg, kreg0 = ["k7"], |
There was a problem hiding this comment.
I find the kreg0 register group name quite surprising. Intuitively I expected this register class to include just the k0 register, rather than being kreg + k0. In retrospective it makes sense, but I felt there was definitely a surprise factor of sorts. I wonder if there is a name which would have an obvious interpretation.
As a side note, preexisting, and probably half my fault, but isn't the naming of this group inconsistent with the other groups? We can't fix this now, can we?
There was a problem hiding this comment.
I don't have a better ideas for the register class name.
I don't think the naming is too inconsistent. kreg is what you usually want most of the time.
There was a problem hiding this comment.
I don't have a better ideas for the register class name.
Hmm… kreg_full or kreg_all, perhaps?
I don't think the naming is too inconsistent. kreg is what you usually want most of the time.
Ah, I meant in relation to other classes like abcd_reg, xmm_reg, ymm_reg et al. If the convention of separating reg from types of register, kreg would be k_reg. Nothing we can do here anymore, though.
There was a problem hiding this comment.
The rule is consistent but idiosyncratic: kreg, vreg, preg, then xmm_reg, x87_reg, etc.
Like
if class.len() <= 1 {
format!("{class}reg")
else {
format!("{class}_reg")
}
|
I think I'll just go with the most forward-compatible approach which is just to let |
|
This avoids the naming issue since a clobber-only register class can't be used directly. |
This comment has been minimized.
This comment has been minimized.
Previously we only exposed a kreg register class which excludes the k0 register since it can't be used in many instructions. However k0 is a valid register and we need to have a way of marking it as clobbered for clobber_abi. Fixes rust-lang#94977
|
@bors r+ I guess the takeaway here I have is that we probably want a mechanism to feature-gate specific register classes so that we could introduce additional ones for stable inline-asm implementations without making them insta-stable too. |
|
📌 Commit b2bc469 has been approved by |
bors
added
S-waiting-on-bors
Rollup of 6 pull requests Successful merges: - rust-lang#95740 (asm: Add a kreg0 register class on x86 which includes k0) - rust-lang#95813 (Remove extra space before a where clause) - rust-lang#96029 (Refactor loop into iterator; simplify negation logic.) - rust-lang#96162 (interpret: Fix writing uninit to an allocation) - rust-lang#96165 (Miri provenance cleanup) - rust-lang#96205 (Use futex locks on emscripten.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This was added by rust-lang/rust#95740.
asm: Add a kreg0 register class on x86 which includes k0 Previously we only exposed a kreg register class which excludes the k0 register since it can't be used in many instructions. However k0 is a valid register and we need to have a way of marking it as clobbered for clobber_abi. Fixes rust-lang#94977
Previously we only exposed a kreg register class which excludes the k0
register since it can't be used in many instructions. However k0 is a
valid register and we need to have a way of marking it as clobbered for
clobber_abi.
Fixes #94977