[PATCH] D83127: [InstCombine] canEvaluateTruncated - use KnownBits to check for inrange shift amounts

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:398
+        llvm::computeKnownBits(I->getOperand(1), IC.getDataLayout());
+    APInt ShiftedBits = APInt::getBitsSetFrom(OrigBitWidth, BitWidth);
+    if (AmtKnownBits.getMaxValue().ult(BitWidth) &&
----------------
RKSimon wrote:
> lebedev.ri wrote:
> > Shouldn't this take `AmtKnownBits.getMaxValue()` into account?
> > 
> > I.e. if we are truncating `i32 -> i16`, but are shifting by at most `1`,
> > we don't need for all bits under mask `0xFFFF0000` to be zeros,
> > we only need that for the bits we'd potentially shift-in,
> > i.e. only `0x10000`. Or am i missing something?
> You're right - this is an existing issue - do you want me to deal with that first?
No, but i'd like to see a FIXME.
It, and the same improvement in `ashr` handling can be done as follow-ups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83127