github-actions[bot] commented on Issue #2051:
Subscribe to Label Action
cc @bnjbvr
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64"Thus the following users have been cc'd because of the following labels:
- bnjbvr: cranelift
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
cfallin commented on Issue #2051:
@julian-seward1 I reworked the negation/casting a bit as per our discussion -- PTAL!
julian-seward1 commented on Issue #2051:
Thanks for the simplification. It is simpler, but tbh I am still unclear about the need for the masking. I had in mind the following; would it work?
assert!(twos_complement_bits <= 64); if let Some(imm_value) = input_to_const(ctx, input) { let mut sign_extended = ((imm_value as i64) << (64 - twos_complement_bits)) >> (64 - twos_complement_bits); if let Some(i) = Imm12::maybe_from_u64(sign_extended as u64) { return (ResultRSEImm12::Imm12(i), false); } sign_extended = sign_extended.wrapping_neg(); if let Some(i) = Imm12::maybe_from_u64(sign_extended as u64) { return (ResultRSEImm12::Imm12(i), true); } } ( ResultRSEImm12::from_rse(put_input_in_rse(ctx, input, narrow_mode)), false, )
cfallin commented on Issue #2051:
Ah, yes, I just realized that the masking doesn't actually matter: the
Imm12
's limit is lower than the maximum value for the masked-down width, so masking to the appropriate width first will not enable any additional constant values to be represented. (In thei8
case where that's not true, any negative value would also fit without negation.)
cfallin commented on Issue #2051:
Thanks! Updated naming to
put_input_in_rse_imm12_maybe_negated
; hopefully clearer.Re: the sign extension: one way to think of it is that the sign extension does not affect the correctness of the add/sub, because narrower-than-64-bit values have undefined high bits. (So even adding 127 == -128 in
i8
-world is safe, because it produces the correct lowest byte.) The high bytes only matter whereImm12
is concerned -- it may falsely decide we can't use a 12-bit immediate because (actually ignored) high bits are set. So we need to sign extend before doing the 64-bit negation, so that we get a sign-extended positive value and can recognize small negative numbers as representable. (And why not mask the first, non-negated, case then? Because the constant withiconst
should already be within range if positive.)
Last updated: Dec 23 2024 at 12:05 UTC