Cranelift(Aarch64): Optimize lowering of icmps with immediates
#5252
Conversation
| ; movz w10, #65512 | ||
| ; subs xzr, x9, x10 | ||
| ; b.ls label1 ; b label2 | ||
| ; block1: | ||
| ; add x11, x0, x1, UXTW | ||
| ; add x11, x11, #16 | ||
| ; movz x10, #65512 | ||
| ; movz w10, #65512 |
There was a problem hiding this comment.
I'm not sure why these changed here, but also I think it is benign since movz is zero-extending the value?
|
Here is an example of what we used to lower these to (i.e. what happens when I comment out the rules that implement these rewrites): --- expected
+++ actual
@@ -1,4 +1,6 @@
block0:
- subs wzr, w0, #1118208
- cset x0, hi
+ movz w2, #4097
+ movk w2, w2, #17, LSL #16
+ subs wzr, w0, w2
+ cset x0, hs
ret |
github-actions
bot
added
cranelift
We can encode more constants into 12-bit immediates if we do the following
rewrite for comparisons with odd constants:
A >= B + 1
==> A - 1 >= B
==> A > B
(And similar for less-than-or-equals.)
fitzgen
force-pushed
the
optimize-comparisons-with-immediates-on-aarch64
branch
from
d1f5fef to
5e03b65
Compare
|
For those following along at home: the test failure from the previous iteration of this PR was because the rules previously implemented for this "same" optimization for turning less-than-or-equal into less-than was incorrect (it would apply if the constant was on the LHS, but was firing when the constant was on the RHS). In the future, we could try and "rotate" |
| fn make_imm12_from_u64(&mut self, n: u64) -> Option<Imm12> { | ||
| Imm12::maybe_from_u64(n) | ||
| } | ||
|
|
There was a problem hiding this comment.
Why is make_imm12_from_u64 separate from imm12_from_u64? They look identical to me.
There was a problem hiding this comment.
One is a pure constructor and the other is a fallible extractor. They are identical but can't replace one another in ISLE's type system.
There was a problem hiding this comment.
Couldn't it be an infallible extractor? Then there'd be no Option in the return type, and the same rust function could be reused for the extractor and constructor in isle.
There was a problem hiding this comment.
Scratch that -- I just realized that we're relying on the failure behavior in the extractor case :)
There was a problem hiding this comment.
It can't be infallible because not all u64s can be represented with Imm12s
edit: yes, exactly
There was a problem hiding this comment.
Ah, I see. But then you could have used the extractor instead of a constructor, right?
(if-let (imm12_from_u64 imm) (u64_sub b 1))
We can encode more constants into 12-bit immediates if we do the following rewrite for comparisons with odd constants:
(And similar for less-than-or-equals.)