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

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

lgtm modulo minor nits inline


================
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())) {
----------------
reames wrote:
> 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.  
I ended up doing this today: http://reviews.llvm.org/rL251180 so you should be able to change this to just use `getConstantRangeFromMetadata`.

================
Comment at: lib/Analysis/LazyValueInfo.cpp:573
@@ +572,3 @@
+  // If this is an instruction which supports range metadata, return the
+  // implied range.  Note: This should be an intersection, not a union.
+  Res.mergeIn(getFromRangeMetadata(BBI), DL);
----------------
By "should be" do you mean at some point in the future?  Because AFAICT today `mergeIn` will do a union.

If you do mean "at some point in the future", please make that obvious, perhaps by changing this to a TODO or a FIXME.


http://reviews.llvm.org/D13543