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

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev 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)
----------------
spatel wrote:
> anton-afanasyev wrote:
> > spatel wrote:
> > > We already returned if MinBitWdith based on KnownRHS was too big, so this std::max is redundant?
> > No, it isn't: we still use this updated MinBitWidth by setting it to instruction Info:
> > ```
> > Itr.second.MinBitWidth = MinBitWidth;
> > ```
> > This value is then used while computing common MinBitWidth in getMinBitWidth() function.
> Ah, ok. Do we have a test to exercise that path? I didn't see any test failures when I made the change locally.
Ok, added test to cover that path. With your change we get incorrect transformation: https://alive2.llvm.org/ce/z/fZ6jFb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108201