[PATCH] Add support for nonnull metadata on Loads

Pete Cooper peter_cooper at apple.com
Wed Oct 22 14:45:35 PDT 2014


> On Oct 22, 2014, at 2:42 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> 
> On 10/22/2014 11:17 AM, Pete Cooper wrote:
>> Hey Philip
>> 
>> Sorry to go back to a committed patch, but it seemed like the best place for these questions.
> Questions and comments are always welcome.
Thanks :)
>> 
>> So firstly I really like the patch here, I have no problems with it.
>> 
>> I’m curious what your plans are for this in terms of expanding it to other instructions.  I’d like to see this on PHIs for example.
> I'm not planning to do so currently.  The philosophy around metadata appears to be that we add it only to "source" instructions - those which can introduce a value from the outside world.  A PHI (and most other instructions) can be analysed and the metadata isn't needed.
Good point.
> 
> If you can come up with a strong case which would motivate metadata on a phi, I'd be interested in seeing it.
Only in that putting the metadata on a PHI would essentially cache the result to avoid recalculating it.
>> 
>> You also mentioned call returns.  I think that this complements function return attributes as I could do something like this:
>> 
>> %0 = load … !nonnull
>> call foo(%0)
>> 
>> void* foo(void*p) { return p }
>> 
>> You can’t put the nonnull attribute on the function return itself as you can’t prove that its always nonnull for all callers, but your metadata can be put on a specific call site like the one above.
> You can put the nonnull attribute on the return value of the callsite.  (I think.  I haven't actually checked.)  Given this, the metadata form would be redundant.
> 
Ah, you’re right.  I thought the attributes on a call were actually just those on the function itself, but it looks like CallInst has its own AttributeSet.
> 
>> 
>> Finally, any plans for clang to emit these?  vtable reads for example are probably guaranteed to be nonnull…probably :)
> I'm not working on clang, but if someone else wanted to do so, I'd encourage it.
Cool.  I don’t have any immediate plans, but if I do I’ll let you know.

Thanks,
Pete
>> 
>> Thanks,
>> Pete
>>> On Oct 20, 2014, at 3:51 PM, Philip Reames <listmail at philipreames.com> wrote:
>>> 
>>> Closed by commit rL220240 (authored by @reames).
>>> 
>>> REPOSITORY
>>>  rL LLVM
>>> 
>>> http://reviews.llvm.org/D5220
>>> 
>>> Files:
>>>  llvm/trunk/docs/LangRef.rst
>>>  llvm/trunk/lib/Analysis/ValueTracking.cpp
>>>  llvm/trunk/test/Transforms/InstSimplify/compare.ll
>>> <D5220.15157.patch>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 





More information about the llvm-commits mailing list