[PATCH] Add support for nonnull metadata on Loads

Philip Reames listmail at philipreames.com
Fri Oct 17 14:28:58 PDT 2014


>>! In D5220#12, @hfinkel wrote:
>> I chose not to adopt the name result.nonnull. I believe we should stick with precedent here. There's an existing nonnull attribute with the same semantics. I believe using an alternate name would be more confusing.
> 
> I hate to be a pain, but -- now that I've had time to think about it ;) -- I *really* would like to name this something that makes it clear that the semantics apply to the result, and not the operand -- we *don't* currently have a naming precedent here (for metadata on loads that mirrors argument attributes), and I'd like to set a precedent that makes sense, not one that will lead to confusion. The existing attributes are clearly attached to values, so there is no ambiguity. But here there is a large ambiguity: there are two pointers involved here, and seeing !nonnull next to a load, it would be perfectly reasonable to assume that meant that the address being loaded from is nonnull (and not that the pointer being loaded is nonnull). Let's pick a less-ambiguous name and I think this is good. Let's set a precedent for result.<attr-name> or value.<attr-name> or whatever, I think that will be much better.

I would appreciate inputs from others on this point.

I don't agree with Hal's points.  I don't really see there being much confusion - all metadata and most attributes apply to the value of the instruction, not the operands - and I believe that having two names for the exact same semantics (one attribute, one metadata) is confusing.  

If outvoted, I will go along with the majority here, but I believe the naming scheme Hal is proposing would be a mistake.

http://reviews.llvm.org/D5220






More information about the llvm-commits mailing list