[PATCH] Add support for nonnull metadata on Loads

[Hal Finkel]

----- Original Message -----
> From: "Philip Reames" <listmail at philipreames.com>
> To: "Hal Finkel" <hfinkel at anl.gov>, reviews+D5220+public+15c0a768ddf63cc0 at reviews.llvm.org
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Monday, September 8, 2014 1:21:45 PM
> Subject: Re: [PATCH] Add support for nonnull metadata on Loads
> 
> 
> On 09/08/2014 04:32 AM, Hal Finkel wrote:
> > ----- Original Message -----
> >> From: "Philip Reames" <listmail at philipreames.com>
> >> To: listmail at philipreames.com, hfinkel at anl.gov
> >> Cc: llvm-commits at cs.uiuc.edu
> >> Sent: Friday, September 5, 2014 4:42:25 PM
> >> Subject: [PATCH] Add support for nonnull metadata on Loads
> >>
> >> Hi hfinkel,
> >>
> >> This patch simply adds support for marking a load as returning a
> >> non-null pointer.  This is analogous to the existing nonnull
> >> return
> >> attribute, but it applies to specific load instructions instead.
> >>   The value of the load is allowed to vary between successive
> >>   loads,
> >> but null is not a valid value to be loaded by any load marked
> >> nonnull.
> >>
> >> Hal - I'd particularly like your input here.  Is adding a parallel
> >> metadata construct to the existing attribute a reasonable idea?
> >>   Should we be uniform and accept the metadata on calls too?  Is
> >> there a better way to approach this.
> > Now that the @llvm.assume infrastructure is in place, I can
> > legitimately respond by saying: I think that we can model this by
> > @llvm.assume(x != null). If that currently does not work, then we
> > should fix it.
> A couple of counter points:
> 1) We already use metadata for optimization hints in multiple ways.
> It
> works.  While having this represented via assumes is nice, I don't
> want
> to block forward progress in a search for perfection.
> 2) I don't think assumes and metadata should be considered an
> either-or.  If something is widely useful enough, having it supported
> by
> metadata is actually a good thing.  As we've discussed previously,
> using
> assumes has some downsides.
> 3) If anything, having assume(x != null) canonicalize to a nonnull
> attribute or metadata (depending on the type of x), would seem like a
> very reasonable implementation.

I agree with all of this, but let's not prematurely optimize. The assume technique should work now, and if it becomes clear that using it has too much overhead, then using metadata seems like the next logical choice.

 -Hal

> 
> Philip
> >> http://reviews.llvm.org/D5220
> >>
> >> Files:
> >>    lib/Analysis/ValueTracking.cpp
> >>    test/Transforms/InstSimplify/compare.ll
> >>
> 
> 

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