[PATCH] [IR] Introduce a dereferenceable_or_null(N) attribute.

Sanjoy Das sanjoy at playingwithpointers.com
Mon Apr 13 14:32:36 PDT 2015


================
Comment at: docs/LangRef.rst:1017
@@ +1016,3 @@
+    This indicates that the parameter or return pointer is either
+    ``dereferenceable(<n>)`` or ``null`` or both.  This attribute may
+    only be applied to pointer typed parameters.
----------------
hfinkel wrote:
> Please explicitly note that it can only be both when not in addrspace(0). When not in addrspace(0), does this attribute imply that null is not dereferenceable (even if it would be otherwise)?
> 
> On the other hand, if dereferenceable_or_null(n) is equivalent to dereferenceable(n) for non-addrspace(0) pointers, maybe we should disallow it to avoid confusion.
> 
> Please explicitly note that it can only be both when not in addrspace(0).

Done.

> When not in addrspace(0), does this attribute imply that null is not dereferenceable (even if it would be otherwise)?

No, it just means that the pointer is at least one of "null" or "dereferenceable(<n>)" (I've changed the language to make this clearer).  So you cannot replace "load i32, i32 aspace(42)* T" where T is known to be "null" and also deref_or_null with "unreachable".

================
Comment at: lib/AsmParser/LLParser.cpp:1557
@@ +1556,3 @@
+///   ::= 'dereferenceable_or_null' '(' 4 ')'
+bool LLParser::ParseOptionalDereferenceableOrNullBytes(uint64_t &Bytes) {
+  Bytes = 0;
----------------
hfinkel wrote:
> Please refactor this so it shares common logic with dereferenceable(n).
Done!

================
Comment at: lib/IR/Attributes.cpp:307
@@ -293,1 +306,3 @@
 
+  if (hasAttribute(Attribute::DereferenceableOrNull)) {
+    std::string Result;
----------------
hfinkel wrote:
> We now have three essentially-identical blocks of code here. Please add some lambda function and combine them instead.
Done!

================
Comment at: test/Assembler/deref-or-null.ll:1
@@ +1,2 @@
+; RUN: llvm-as < %s | llvm-dis | FileCheck %s
+
----------------
hfinkel wrote:
> Don't add another separate test for this. Just add this to test/Bitcode/attributes.ll (along with the tests for dereferenceable).
> 
Done!

http://reviews.llvm.org/D8650

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list