[PATCH] D108201: [AggressiveInstCombine] Add logical shift right instr to `TruncInstCombine` DAG
Anton Afanasyev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 18 11:14:30 PDT 2021
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
More information about the llvm-commits
mailing list