[PATCH] D108201: [AggressiveInstCombine] Add logical shift right instr to `TruncInstCombine` DAG

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:297-298
+        KnownBits KnownLHS = computeKnownBits(I->getOperand(0), DL);
+        MinBitWidth =
+            std::max(MinBitWidth, KnownLHS.getMaxValue().getActiveBits());
+        if (MinBitWidth >= OrigBitWidth)
----------------
We already returned if MinBitWdith based on KnownRHS was too big, so this std::max is redundant?


================
Comment at: llvm/test/Transforms/AggressiveInstCombine/trunc_shifts.ll:163
   %zext = zext i16 %x to i32
   %lshr = lshr i32 %zext, 16
   %trunc = trunc i32 %lshr to i16
----------------
anton-afanasyev wrote:
> spatel wrote:
> > Similar to the previous `shl` example - this is another simplification to 0 that isn't handled by -instsimplify (but regular -instcombine gets it).
> Ok, but here it is simple negative test, checking we do not fold to poisonous `lshr i16 x, 16`.
Yes, not a problem - just pointing out that there's another opportunity to improve InstSimplify and run some kind of cleanup in this pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108201