[PATCH] Add a dereferencable attribute

[Hal Finkel]

----- 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)?

 -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