[PATCH] D47113: [CVP] Teach CorrelatedValuePropagation to reduce the width of lshr instruction.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

@jlebar thank you for looking at this!



================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:604
+  APInt RHS;
+  // If the shift is constant, then we can just use said constant.
+  if (auto *RHSConst = dyn_cast<ConstantInt>(Instr->getOperand(1)))
----------------
jlebar wrote:
> getConstantRange doesn't do the right thing when Value is a constant?
Hm, it seems it does. I suppose this was a premature optimization :)


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:613
+  // Could happen if data type is i64, and there is no data on constant range.
+  if (RHS.isAllOnesValue())
+    return false;
----------------
jlebar wrote:
> I don't think this is sufficient to ensure that RHS.getZExtValue() below doesn't assert?  (For example, RHS could be an int128, as I read the langref.)
Nice catch!


Repository:
  rL LLVM

https://reviews.llvm.org/D47113