[PATCH] Add a dereferencable attribute

[Philip Reames]

On 07/11/2014 04:44 PM, Hal Finkel wrote:
> ----- Original Message -----
>> From: "Philip Reames" <listmail at philipreames.com>
>> To: hfinkel at anl.gov, chandlerc at gmail.com, aschwaighofer at apple.com, nlewycky at google.com, me+llvm at luqman.ca,
>> listmail at philipreames.com
>> Cc: jvoung at chromium.org, jfb at chromium.org, "james molloy" <james.molloy at arm.com>, llvm-commits at cs.uiuc.edu
>> Sent: Friday, July 11, 2014 4:27:10 PM
>> Subject: Re: [PATCH] Add a dereferencable attribute
>>
>> I'm happy with the new encodings.
> Good, I like it better too :-)
>
>>   Some of my previous comments still
>> stand (naming, comments, etc..), but the overall structure looks
>> good.
> Alright, I want to be clear about this, because I thought that I had addressed your comments...
>
>   - I will commit separately the AlignAttr -> IntAttr renaming
>
>   - Regarding getDereferenceableBytes -> getBytesKnownSafeToDereference (or similar), I'm not thrilled by the idea of making a long name even longer, and I think that Dereferenceable implies safety. Would getDereferenceableByteCount be better?
The point I was getting at is that 'dereferencable bytes' could be 
misread.  Right now the name doesn't distinguishing between bytes which 
are *known to be safe to dereference* and bytes which *might be safe to 
dereference*.  The attribute implements the first, but unless you 
already know that, the name isn't clear.  Reading in isolation, either 
interpretation would be reasonable.  The documentation addresses this, 
but looking at code could cause confusion.

Honestly, this is a fairly minor point now that the documentation has 
been improved.

>
>   - If there is anything else under "naming", please clarify.
That was it.
>
>   - There is now some note on every getDereferenceableBytes function stating that zero is returned when the answer is unknown, and the LangRef text has been updated based on your comments.
>
>   - Is there anything else under 'etc.'?
Reading back over my original comments, no.  I'd slightly prefer a 
clearer name, but not enough to hold a submit.  LGTM.
>
>> I am uncomfortable with the coupling of dereferenceable and nonnull.
>>   I don't have an actual counter example, but could we separate that
>> from the current patch and revisit it separately?
> We could, but it would cause a lot of test-case churn (on the Clang side) and I'd rather not. Realistically, the fact that dereferenceablility imples nonnull (only for addrspace(0)) is baked into several pieces of LLVM and specified in the language reference.
I'll accept that argument.
>
> Thanks again,
> Hal
>
>> Thinking out loud, I wonder about a truly perverse runtime system
>> that wanted to use loads from null+offset to represent calls to
>> runtime functions.  The signal handler could advance the PC and
>> parse the load instruction to "return" a value in the right
>> register.  This would be a "dereferenceable" load (very
>> indirectly!), but not non-null.  I don't think you could represent
>> this in LLVM for other reasons, but it's the best I could do on the
>> spot.  :)
>>
>> http://reviews.llvm.org/D4449
>>
>>
>>