[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