[PATCH] Add a dereferencable attribute

[Hal Finkel]

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Nick Lewycky" <nlewycky at google.com>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>, "reviews+d4449+public+728896e6e6399a87"
> <reviews+D4449+public+728896e6e6399a87 at reviews.llvm.org>
> Sent: Thursday, July 10, 2014 10:42:16 PM
> Subject: Re: [PATCH] Add a dereferencable attribute
> 
> ----- Original Message -----
> > From: "Hal Finkel" <hfinkel at anl.gov>
> > To: "Nick Lewycky" <nlewycky at google.com>
> > Cc: "Chandler Carruth" <chandlerc at gmail.com>, "aschwaighofer"
> > <aschwaighofer at apple.com>, "Philip Reames"
> > <listmail at philipreames.com>, "me+llvm" <me+llvm at luqman.ca>, "Commit
> > Messages and Patches for LLVM"
> > <llvm-commits at cs.uiuc.edu>, "reviews+d4449+public+728896e6e6399a87"
> > <reviews+D4449+public+728896e6e6399a87 at reviews.llvm.org>
> > Sent: Thursday, July 10, 2014 8:40:35 PM
> > Subject: Re: [PATCH] Add a dereferencable attribute
> > 
> > ----- Original Message -----
> > > From: "Nick Lewycky" <nlewycky at google.com>
> > > To: "reviews+d4449+public+728896e6e6399a87"
> > > <reviews+D4449+public+728896e6e6399a87 at reviews.llvm.org>
> > > Cc: "Hal Finkel" <hfinkel at anl.gov>, "Chandler Carruth"
> > > <chandlerc at gmail.com>, "aschwaighofer"
> > > <aschwaighofer at apple.com>, "Philip Reames"
> > > <listmail at philipreames.com>, "me+llvm" <me+llvm at luqman.ca>,
> > > "Commit
> > > Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> > > Sent: Thursday, July 10, 2014 5:06:08 PM
> > > Subject: Re: [PATCH] Add a dereferencable attribute
> > > 
> > > 
> > > 
> > > 
> > > On 10 July 2014 14:13, hfinkel at anl.gov < hfinkel at anl.gov > wrote:
> > > 
> > > 
> > > Updated per Nick's comment to include an explicit size (number of
> > > bytes). I currently picked 12 bits for the size, I'm not sure if
> > > that is a good choice (I did not want to use too many bits, are
> > > there only 64 in total?).
> > > 
> > > Also, I renamed the AlignAttr class to IntAttr (along with all
> > > associated functions) because these are now just integer-carrying
> > > attributes (not specifically some kind of alignment).
> > > 
> > > 
> > > http://reviews.llvm.org/D4449
> > > 
> > > 
> > > "dereferencable" vs. "dereferenceable". I think LLVM is
> > > consistently
> > > using the latter right now, your patch consistently uses the
> > > former.
> > > Please fix, either way.
> > > 
> > > 
> > > You know how we never print "readonly readnone" because instead
> > > we
> > > just print "readnone"? (This is reflected in the API as well,
> > > querying for "onlyReadsMemory" returns true if readnone is set
> > > and
> > > readonly isn't, or if readonly is.) What do you think of doing
> > > this
> > > for "nonnull dereferencable(n)"? The only gotcha is that it only
> > > works when the pointer is in address space zero, in non-zero
> > > address
> > > spaces dereferenceable(n) does not imply nonnull.
> > > 
> > 
> > I don't see anything in the library that prevents us from printing
> > it, but you're right that onlyReadsMemory checks both. Implementing
> > something similar for nonnull (and then not adding both in the
> > frontend) can be just as easily done.
> 
> On that thought, are you sure that a reference implies nonnull at all
> when not in addrspace(0)?

I think that I've now convinced myself that nonnull on references must only apply to addrspace(0). If in some other address space you can have an object at address zero, then you can have a reference to it too.

 -Hal

> 
>  -Hal
> 
> > 
> >  -Hal
> > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > Files:
> > > docs/LangRef.rst
> > > include/llvm-c/Core.h
> > > include/llvm/Bitcode/LLVMBitCodes.h
> > > include/llvm/IR/Argument.h
> > > include/llvm/IR/Attributes.h
> > > include/llvm/IR/CallSite.h
> > > include/llvm/IR/Function.h
> > > include/llvm/IR/Instructions.h
> > > lib/AsmParser/LLLexer.cpp
> > > lib/AsmParser/LLParser.cpp
> > > lib/AsmParser/LLParser.h
> > > lib/AsmParser/LLToken.h
> > > lib/Bitcode/Reader/BitcodeReader.cpp
> > > 
> > > 
> > > 
> > > 
> > > else if (Kind == Attribute::Dereferencable)
> > > B.addDereferencableAttr(Record[++i]);
> > > else
> > > llvm_unreachable("Unknown integer attribute type");
> > > 
> > > 
> > > I'm not sure that the appropriate use of llvm_unreachable. It can
> > > be
> > > reached with a crafted .bc file, right?
> > > 
> > > 
> > > lib/Bitcode/Writer/BitcodeWriter.cpp
> > > lib/IR/AttributeImpl.h
> > > 
> > > 
> > > lib/IR/Attributes.cpp
> > > 
> > > 
> > > 
> > > unsigned AttributeSetNode::getDereferencableBytes() const {
> > > for (iterator I = begin(), E = end(); I != E; ++I)
> > > 
> > > 
> > > Range-based for loop?
> > > 
> > > 
> > > 
> > > 
> > > lib/IR/Function.cpp
> > > lib/IR/Value.cpp
> > > test/Bitcode/attributes.ll
> > > test/Transforms/LICM/hoist-deref-load.ll
> > > 
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

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