[PATCH] Add a dereferencable attribute

[Hal Finkel]

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

Yes, it should just be skipped I suppose (so that we ignore unknown attributes).

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

Yes, although I think we also need to add a const_iterator type, and I might as well do that and convert all of the nearby functions together in a follow-up commit.

 -Hal

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