[PATCH] D13543: [LVI/CVP] Teach LVI about range metadata

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:502
@@ +501,3 @@
+static LVILatticeVal getFromRangeMetadata(Instruction *BBI) {
+  switch (BBI->getOpcode()) {
+  default: break;
----------------
chenli wrote:
> Do we actually need to check the Opcode? For instructions those cannot have range metadata, getMetadata(LLVMContext::MD_range) will just return a nullptr. If range metadata is extended for other instructions in the future, this switch statement has to be updated as well. I think it might be better to just get rid of the switch. 
I could go either way on this.  The verifier currently enforces these three, but there's no reason the code here needs to.  What do others think?

================
Comment at: lib/Analysis/LazyValueInfo.cpp:507
@@ +506,3 @@
+  case Instruction::Invoke:
+    if (MDNode *Ranges = BBI->getMetadata(LLVMContext::MD_range))
+      if (auto *IType = dyn_cast<IntegerType>(BBI->getType())) {
----------------
sanjoy wrote:
> I've seen this logic repeated in several places now.  I think it's time to refactor out a `ConstantRange getRangeFromMetadata()` helper function.
> 
> However, I think its fine to check this in as is, and then extract out the helper.
Completely agreed.  Will do as a separate follow on change.  


http://reviews.llvm.org/D13543