[PATCH] D63829: [InstCombine] Shift amount reassociation in bittest (PR42399)
Roman Lebedev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jun 30 07:47:01 PDT 2019
lebedev.ri added a comment.
Thanks for taking a look, will post updated patch in a bit.
================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3267-3277
+ if (!match(
+ &I,
+ m_ICmp(Pred,
+ m_OneUse(m_c_And(
+ m_OneUse(m_CombineAnd(
+ m_LogicalShift(m_Value(X), m_Value(XShAmt)),
+ m_Value(XShift))),
----------------
spatel wrote:
> spatel wrote:
> > This would be easier to read for me if we did the simple icmp checks first:
> > if (!I.isEquality() || !match(I.getOperand(1), m_Zero()))
> > return nullptr;
> >
> > And then do a mega-match on only I.getOperand(0).
> >
> > ...although that match raises a question if the one-use checks are balanced:
> > Do we prefer to shift left or right?
> > https://rise4fun.com/Alive/
> > %t0 = lshr i32 %x, 2
> > %t1 = and i32 %t0, %y
> > %t3 = icmp ne i32 %t1, 0
> > =>
> > %v0 = shl i32 %y, 2
> > %v1 = and i32 %v0, %x
> > %t3 = icmp ne i32 %v1, 0
> Incomplete Alive link in previous comment:
> https://rise4fun.com/Alive/9Kp
> ...although that match raises a question if the one-use checks are balanced:
I think they are currently.
We will produce 3 instructions: `icmp` + `and` + `shift`.
Therefore we need to get rid of 2 instructions (since the root `icmp` is always good to go).
Given the nature of the pattern the `icmp` is comparing `and` with `0`, so `and` must go.
And thus we only need to get rid of one more instruction.
We know that both hands of an `and` are shifts.
Since the pattern is bi-directional, we don't really care which one of these two shifts
will be left, only one of those 2 shifts *has* to be one-use.
And that's what is currently enforced via `m_OneUse()`.
> Do we prefer to shift left or right?
I'm not sure we can answer that here.
If we can do this fold as per one-use checks - we should.
Do you have anything specific in mind here?
I'd say this should be a separate fold.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63829/new/
https://reviews.llvm.org/D63829
More information about the llvm-commits
mailing list