[PATCH] D139598: [InstCombine] Fold (X << Z) / (X * Y) -> (1 << Z) / Y

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1062
+      return BinaryOperator::CreateUDiv(
+          Builder.CreateShl(ConstantInt::get(Ty, 1), Z), Y);
+  }
----------------
Chenbing.Zheng wrote:
> nikic wrote:
> > Chenbing.Zheng wrote:
> > > RKSimon wrote:
> > > > arsenm wrote:
> > > > > Can you preserve NUW and exact?
> > > > Move the !IsSigned check outside to avoid the cost of match calls?
> > > I preserve exact, but it seems that NUW is unnecessary when Op0 of SHL is 1 ?
> > You're right that we can infer nuw for `1 << x`, but based on your tests it doesn't look like we actually do this...
> Yer, I think 'shl nuw 1 << x' is equivalent to 'shl 1 << x', so I tend to don't  keep 'NUW'.  Or what is the benefit of keeping 'NUW' ?
It could enable some other transform. Some tests actually improve with that:
d4493dd1ed58

So assuming that patch is good, it won't matter much now if it isn't included here, but it's still more efficient to create the 'nuw' directly in this patch instead of relying on another fold to do it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139598/new/

https://reviews.llvm.org/D139598