[PATCH] D108091: [AggressiveInstCombine] Add shift left instruction to `TruncInstCombine` DAG

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:282
+    Instruction *I = Itr.first;
+    if (I->getOpcode() == Instruction::Shl) {
+      KnownBits KnownRHS = computeKnownBits(I->getOperand(1), DL);
----------------
anton-afanasyev wrote:
> spatel wrote:
> > This logic is getting more complicated in D108201, so I want to step back to this patch to check my understanding:
> > Can SrcBitWidth be different than OrigBitWidth?
> > If so, can we write a test for that?
> > If not, then assert that I->getOperand(1)->getType()->getScalarSizeInBits == OrigBitWidth. Then simplify this code to something like:
> >       KnownBits KnownRHS = computeKnownBits(I->getOperand(1), DL);
> >       unsigned MinBitWidth = KnownRHS.getMaxValue()
> >                                  .uadd_sat(APInt(OrigBitWidth, 1))
> >                                  .getZExtValue();
> >       if (MinBitWidth >= OrigBitWidth)
> >         return nullptr;
> > 
> Yes, `SrcBitWidth` can be different than `OrigBitWidth` (smaller or larger). `OrigBitWidth` is bitwidth of original (dominating) `trunc` operand (before truncation) whereas `SrcBitWidth` is bitwidth of shift instruction. Added tests for such cases: https://reviews.llvm.org/rG0988488ed461
Btw, I was wrong here: actually `SrcBitWidth == OrigBitWidth`, since `zext`, `sext` and `trunc` can only be leaves of `trunc`-dominated DAG, so `OrigBitWidth` is bitwidth of all DAG's instructions. I've removed all appropriate tests since they all were positive and didn't checked anything. (fixed in the last patch here https://reviews.llvm.org/D108355)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108091