[PATCH] [IC] Turn non-null MD on pointer loads to range MD on integer loads.

Chandler Carruth chandlerc at gmail.com
Mon Feb 23 18:05:12 PST 2015


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:351-353
@@ -343,5 +350,5 @@
 
     case LLVMContext::MD_range:
       // FIXME: It would be nice to propagate this in some way, but the type
       // conversions make it hard.
       break;
----------------
cdavis5x wrote:
> chandlerc wrote:
> > cdavis5x wrote:
> > > chandlerc wrote:
> > > > Do you want to handle the inverse in this patch? That would then make for a nice round-trip test that we can go from pointer -> int -> pointer and preserve non-null.
> > > That would be nice, but I think I'd like to be conservative in this patch for now. I'll see about turning `!range` MD into `!nonnull` MD later.
> > > 
> > > Besides, this raises some questions of its own, like: if we're converting an integer load to a pointer load, and the integer load has a `!range` other than `!{iPTR 1, iPTR -1}` (but that still doesn't include 0, or whatever `ptrtoint null` is), should we then convert the `!range` to `!nonnull` MD? (I think we should, but the problem is that we lose some information there. I suspect this is part of why you held off on that. :)
> > This really raises a whole host of problems.
> > 
> > Is it even correct to translate !nonnull -> an integer range that doesn't include zero? Do we guarantee that ptrtoint always produces a zero integer for a null pointer? I don't think we do. And in practice, I don't think that is always safe.
> > 
> > I'm starting to think this entire endeavor is not safe, and we need to take several steps back.
> All right. Want me to just abandon this for now?
No, we've been debating thin in the IRC channel.

We think this is all "ok". What we should do is compute the range by computing constantexprs with the null pointer and ptrtoint. Essentially, define the range as ["(add (ptrtoint null) 1)", "(ptrtoint null)"). And we can use the same address space of null pointer for the ptrtoint, and this code will be no more or less wrong than the constant folder.

For the handling the reverse case, we just check whether (ptrtoint null) is outside of the range, and if so, synthesize !nonnull.

================
Comment at: test/Transforms/InstCombine/loadstore-metadata.ll:98
@@ -99,2 +97,3 @@
 
+; CHECK: ![[MD]] = !{i64 1, i64 -1}
 !0 = !{ !1, !1, i64 0 }
----------------
cdavis5x wrote:
> chandlerc wrote:
> > Does this work even if the above test case isn't the last one in the file? I wonder if we need to make the IR printing denormalize this somewhat for easier checking.
> Probably not, given that it's a plain ol' `CHECK` line. If any more `CHECK` lines are added after this, they will fail, because `FileCheck` will have already scanned past them.
> 
> Frankly, I was kind of worried about that when I wrote this. (I should've brought it up then...) I considered using `CHECK-DAG`, but I think that only works between the previous and next `CHECK-LABEL` lines. We might need to do what you said and have the IR pretty printer emit MD tuples immediately after the function that uses them (for instance) rather than always at the end of the file.
Are you planning to look into that?

Either way, can you leave a comment here to help the next person to debug this?

http://reviews.llvm.org/D7621

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list