[PATCH] Add support for nonnull metadata on Loads

[Hal Finkel]

----- Original Message -----
> From: "Philip Reames" <listmail at philipreames.com>
> To: listmail at philipreames.com, hfinkel at anl.gov
> Cc: nlewycky at google.com, llvm-commits at cs.uiuc.edu
> Sent: Friday, October 17, 2014 4:28:58 PM
> Subject: Re: [PATCH] Add support for nonnull metadata on Loads
> 
> >>! 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.

I'm not proposing two different names, I'm proposing a prefix that is used on the metadata name. Many metadata names have prefixes. Also, I reject your premise:
  !tbaa, !noalias/!alias.scope, !llvm.mem.parallel_loop_access, all apply to loads, and all refer to properties of the pointer being loaded from. Only !range applies to loads and refers to the value being loaded, and I don't see any particular ambiguity there. There are more types of metadata in the first category than in the second (!invariant.load could, perhaps, be argued either way, so I left it out, but I also view it as more of a loaded-from-pointer property than a property of the value being loaded).

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

Sounds good. ;)

Thanks again,
Hal

> 
> http://reviews.llvm.org/D5220
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory