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

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

Seems reasonable to me.  I dunno if this solves @bixia's problem or not, but even if it doesn't, seems reasonable...

Would like a CVP person to approve, though.



================
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)))
----------------
getConstantRange doesn't do the right thing when Value is a constant?


================
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;
----------------
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.)


Repository:
  rL LLVM

https://reviews.llvm.org/D47113