Page MenuHomePhabricator

NFC. Introduce Value::isPointerDereferenceable
ClosedPublic

Authored by apilipenko on Feb 25 2016, 8:44 AM.

Details

Summary

This is analogous to D17572 (and the diff is based upon it). Extract a part of isDereferenceableAndAlignedPointer functionality to Value::isPointerDereferenceable. The difference between those function is that isPointerDereferenceable can be used for opaque types.

Diff Detail

Repository
rL LLVM

Event Timeline

apilipenko updated this revision to Diff 49073.Feb 25 2016, 8:44 AM
apilipenko retitled this revision from to NFC. Introduce Value::isPointerDereferenceable.
apilipenko updated this object.
apilipenko added reviewers: hfinkel, reames.
apilipenko added a subscriber: llvm-commits.
hfinkel added inline comments.Apr 26 2016, 6:31 PM
lib/Analysis/Loads.cpp
96 ↗(On Diff #49073)

Leaving CheckForNull unimplemented seems prone to confusion, and this assert text does not say that this is not yet implemented. Do you just need to call isKnownNonNull? If so, do that.

majnemer added inline comments.
lib/IR/Value.cpp
556 ↗(On Diff #49073)

I'd expect this to call isDereferenceableFromAttribute.

apilipenko updated this revision to Diff 55203.Apr 27 2016, 6:42 AM

Check CheckForNonNull.

apilipenko added inline comments.Apr 27 2016, 6:48 AM
lib/IR/Value.cpp
556 ↗(On Diff #49073)

Can you elaborate? isDereferenceableFromAttribute is an internal function in Loads.cpp, while this is a public API to get inherent property of the value.

sanjoy added a subscriber: sanjoy.Apr 28 2016, 2:39 PM

The overall idea seems reasonable, but isn't this increasing our dependence on pointee types?

@sanjoy, it has nothing to do with pointer types. There are some "kinds" of pointers (e.g. allocas, some globals) which are inherently dereferenceable.

sanjoy accepted this revision.May 2 2016, 2:34 PM
sanjoy added a reviewer: sanjoy.

@sanjoy, it has nothing to do with pointer types. There are some "kinds" of pointers (e.g. allocas, some globals) which are inherently dereferenceable.

I should've been clearer: this is in context of removing pointee types
from LLVM, and having only on pointer type per address space. In that
world, the current trick of "remembering" the number of bytes we want
dereferenceable implicitly via the type of the pointer won't work.

Perhaps a slightly more future proof API would be:

bool isPointerDereferenceable(bool &CanBeNull, unsigned SizeInBytes) const;

so that it can work with (made up syntax):

%ptr = alloca i32
...  ;; ptr is of type pointer addrspace(1), no i32 involved
%val = select <cond>, ..., %ptr
load i16, %val ;; dereferenceable

and

%ptr = alloca i32
...  ;; ptr is of type pointer addrspace(1), no i32 involved
%val = select <cond>, ..., %ptr
load i64, %val  ;; not dereferenceable

However, your patch does not make things any worse, so this is a minor
nit at best, fix at your own discretion.

This revision is now accepted and ready to land.May 2 2016, 2:34 PM
This revision was automatically updated to reflect the committed changes.