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

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.

 -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