[PATCH] D126040: [InstCombine] Fold a mul with bool value into and
Dávid Bolvanský via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 20 04:44:44 PDT 2022
xbolva00 added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:312
+ match(Op1, m_And(m_Value(Y), m_One())))
+ return BinaryOperator::CreateAnd(Op0, Op1);
+ }
----------------
Allen wrote:
> Allen wrote:
> > craig.topper wrote:
> > > xbolva00 wrote:
> > > > grandinj wrote:
> > > > > Surely the more general case is something like:
> > > > >
> > > > > if (ComputeMaxSignificantBits(Op0) ==1 ||
> > > > > ComputeMaxSignificantBits(Op1)==1)
> > > > >
> > > > > ?
> > > > >
> > > > Yeah, please more general case.
> > > ComputeMaxSignificantBits(Op0) ==1 means the input is 0 or -1.
> > >
> > > You would need computeKnownBits or MaskedValueIsZero.
> > Thanks very much , I'll try to use the API ComputeMaxSignificantBits.
> >
> > and there is some failure cases, for example, it 'll turn the -42 into 42.
> > ```
> > ; X * C (when X is a sext boolean) --> X ? -C : 0
> >
> > define i32 @mul_sext_bool(i1 %x) {
> > ; CHECK-LABEL: @mul_sext_bool(
> > ; CHECK-NEXT: [[M:%.*]] = select i1 [[X:%.*]], i32 -42, i32 0
> > ; CHECK-NEXT: ret i32 [[M]]
> > ;
> > %s = sext i1 %x to i32
> > %m = mul i32 %s, 42
> > ret i32 %m
> > }
> > ```
> hi @craig.topper, base your comment, I tried the following change, and it works.
> ```
> + KnownBits XKnown = computeKnownBits(Op0, 0, &I);
> + KnownBits YKnown = computeKnownBits(Op1, 0, &I);
> + if ((XKnown.countMaxPopulation() == 1) &&
> + (YKnown.countMaxPopulation() == 1))
> + return BinaryOperator::CreateAnd(Op0, Op1);
> ```
>
> but there is a new failure case **mul_bools_use3 **in file llvm/test/Transforms/InstCombine/mul.ll. And I can't sure which version is better, should I add extra condition to avoid touch this case ?
> * without above change
> ```
> %r = select i1 %x, i32 %zy, i32 0
> ```
> * with above change
> ```
> %r1 = and i1 %x, %y
> %r = zext i1 %r1 to i32
> ```
>
So then we miss this transformation as well.
%r1 = and i1 %x, %y
%r = zext i1 %r1 to i32
=>
%r = select i1 %x, i32 %zy, i32 0
but which form is better? target specific? @spatel
================
Comment at: llvm/test/Transforms/InstCombine/trunc.ll:1025
+
+define i64 @PR55599_One0(i64 %x, i64 %y) {
+; CHECK-LABEL: @PR55599_One0(
----------------
I dont think trunc.ll file is suitable for these new tests. Find some other like mul.ll or just create new one.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126040/new/
https://reviews.llvm.org/D126040
More information about the llvm-commits
mailing list