Page MenuHomePhabricator

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

Authored by sanjoy on Mar 26 2015, 9:12 PM.

Details

Summary

If a pointer is marked as dereferenceable_or_null(N), LLVM assumes it
is either null or dereferenceable(N) or both. This change only
introduces the attribute and adds a token test case for the llvm-as
/ llvm-dis. It does not hook up other parts of the optimizer to
actually exploit the attribute -- those changes will come later.

For pointers in address space 0, dereferenceable(N) is now exactly
equivalent to dereferenceable_or_null(N) && nonnull. For other
address spaces, dereferenceable(N) is potentially weaker than
dereferenceable_or_null(N) && nonnull (since we could have a null
dereferenceable(N) pointer).

The motivating case for this change is Java (and other managed
languages), where pointers are either null or dereferenceable up to
some usually known-at-compile-time constant offset.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy updated this revision to Diff 22775.Mar 26 2015, 9:12 PM
sanjoy retitled this revision from to [IR] Introduce a dereferenceable_xor_null(N) attribute..
sanjoy updated this object.
sanjoy edited the test plan for this revision. (Show Details)
sanjoy added reviewers: hfinkel, rafael.
sanjoy added a subscriber: Unknown Object (MLST).

For context -- this was discussed on llvmdev some time ago:
http://article.gmane.org/gmane.comp.compilers.llvm.devel/82186

Sanjoy Das wrote:

Hi hfinkel, rafael,

If a pointer is marked as dereferenceable_xor_null(N), LLVM assumes it
is exactly one of { null, dereferenceable(N) }.

Painting the bike shed ... I dislike the name so much I'd rather you
changed "dereferenceable" to mean this, and then allow people to apply
"nonnull" with dereferenceable to get the current meaning of
deferenceable. Changing the meaning of the existing attribute is less
bad. (I also think that it's strict a loosening of the guarantees that
dereferenceable offers, so using older .ll/.bc files with newer llvm
will work, with loss of optimization opportunity in the worst case.
That's fine.)

This change only

introduces the attribute and adds a token test case for the llvm-as /
llvm-dis. It does not hook up other parts of the optimizer to
actually exploit the attribute -- those changes will come later.

For pointers in address space 0, dereferenceable(N) is now exactly
equivalent to dereferenceable_xor_null(N)&& nonnull. For other
address spaces, dereferenceable(N) is potentially weaker than
dereferenceable_xor_null(N)&& nonnull (since we could have a null
dereferenceable(N) pointer).

The motivating case for this change is Java (and other managed
languages), where pointers are either null or dereferenceable up to
some usually known-at-compile-time constant offset.

Also pointers returned by malloc. LibCallSimplifier::optimizeCall
annotates known functions with attributes, please add that there to
anything that it applies to.

http://reviews.llvm.org/D8650

Files:

docs/LangRef.rst
include/llvm-c/Core.h
include/llvm/Bitcode/LLVMBitCodes.h
include/llvm/IR/Attributes.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
lib/Bitcode/Writer/BitcodeWriter.cpp
lib/IR/AttributeImpl.h
lib/IR/Attributes.cpp
lib/IR/Function.cpp
lib/IR/Instructions.cpp
test/Assembler/deref-xor-null.ll

EMAIL PREFERENCES

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

llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

hfinkel edited edge metadata.Mar 30 2015, 5:30 AM
  • Original Message -----

From: "Nick Lewycky" <nicholas@mxc.ca>
To: hfinkel@anl.gov, "rafael espindola" <rafael.espindola@gmail.com>
Cc: nicholas@mxc.ca, llvm-commits@cs.uiuc.edu
Sent: Friday, March 27, 2015 12:24:00 AM
Subject: Re: [PATCH] [IR] Introduce a dereferenceable_xor_null(N) attribute.

Sanjoy Das wrote:

Hi hfinkel, rafael,

If a pointer is marked as dereferenceable_xor_null(N), LLVM assumes
it

is exactly one of { null, dereferenceable(N) }.

Painting the bike shed ... I dislike the name so much I'd rather you

FWIW, I dislike this name as well, specifically the 'xor' part. Honestly, I think that dereferenceable_or_null is clear enough, but I can understand how some might find this confusing. Maybe dereferenceable_unless_null? Or just dereferenceable_if_not_null? A null pointer is address space 0 is not dereferenceable (regardless of this attribute), but what about not in address space zero? Normally, being null does not convey any information about being dereferenceable, but with this attribute, do you intend to change that?

My preference for using a different name was for correspondence with the API. It would be odd, I think, if adding dereferenceable to a pointer value was insufficient to cause isDereferenceablePointer to return true. This is not a super-strong preference ;)

-Hal

changed "dereferenceable" to mean this, and then allow people to
apply
"nonnull" with dereferenceable to get the current meaning of
deferenceable. Changing the meaning of the existing attribute is less
bad. (I also think that it's strict a loosening of the guarantees
that
dereferenceable offers, so using older .ll/.bc files with newer llvm
will work, with loss of optimization opportunity in the worst case.
That's fine.)

This change only

introduces the attribute and adds a token test case for the
llvm-as /

llvm-dis. It does not hook up other parts of the optimizer to

actually exploit the attribute -- those changes will come later.

For pointers in address space 0, dereferenceable(N) is now
exactly

equivalent to dereferenceable_xor_null(N)&& nonnull. For
other

address spaces, dereferenceable(N) is potentially weaker than

dereferenceable_xor_null(N)&& nonnull (since we could have a
null

dereferenceable(N) pointer).

The motivating case for this change is Java (and other managed

languages), where pointers are either null or dereferenceable up
to

some usually known-at-compile-time constant offset.

Also pointers returned by malloc. LibCallSimplifier::optimizeCall
annotates known functions with attributes, please add that there to
anything that it applies to.

http://reviews.llvm.org/D8650

Files:

docs/LangRef.rst
include/llvm-c/Core.h
include/llvm/Bitcode/LLVMBitCodes.h
include/llvm/IR/Attributes.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
lib/Bitcode/Writer/BitcodeWriter.cpp
lib/IR/AttributeImpl.h
lib/IR/Attributes.cpp
lib/IR/Function.cpp
lib/IR/Instructions.cpp
test/Assembler/deref-xor-null.ll

EMAIL PREFERENCES

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

llvm-commits mailing list

llvm-commits@cs.uiuc.edu

http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

http://reviews.llvm.org/D8650

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/
sanjoy updated this revision to Diff 23235.Apr 3 2015, 12:40 PM
sanjoy edited edge metadata.

Rename attribute (and all related things) to dereferenceable_or_null.

  • s/xor/or/
sanjoy retitled this revision from [IR] Introduce a dereferenceable_xor_null(N) attribute. to [IR] Introduce a dereferenceable_or_null(N) attribute..Apr 3 2015, 12:45 PM
sanjoy updated this object.
sanjoy added a comment.Apr 7 2015, 9:11 PM

Super-gentle ping.

hfinkel added inline comments.Apr 11 2015, 5:20 PM
docs/LangRef.rst
1017 ↗(On Diff #23235)

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.

lib/AsmParser/LLParser.cpp
1557 ↗(On Diff #23235)

Please refactor this so it shares common logic with dereferenceable(n).

lib/IR/Attributes.cpp
307 ↗(On Diff #23235)

We now have three essentially-identical blocks of code here. Please add some lambda function and combine them instead.

test/Assembler/deref-or-null.ll
1 ↗(On Diff #23235)

Don't add another separate test for this. Just add this to test/Bitcode/attributes.ll (along with the tests for dereferenceable).

sanjoy added inline comments.Apr 13 2015, 2:32 PM
docs/LangRef.rst
1017 ↗(On Diff #23235)

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

lib/AsmParser/LLParser.cpp
1557 ↗(On Diff #23235)

Done!

lib/IR/Attributes.cpp
307 ↗(On Diff #23235)

Done!

test/Assembler/deref-or-null.ll
1 ↗(On Diff #23235)

Done!

sanjoy updated this revision to Diff 23699.Apr 13 2015, 2:32 PM
sanjoy updated this object.

Address Hal's review.

hfinkel accepted this revision.Apr 16 2015, 12:38 PM
hfinkel edited edge metadata.

LGTM.

test/Bitcode/attributes.ll
249 ↗(On Diff #23699)

Please add the attribute on the return value to provide coverage on that.

This revision is now accepted and ready to land.Apr 16 2015, 12:38 PM
This revision was automatically updated to reflect the committed changes.