<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 3/22/21 12:15 PM, Eric Christopher
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CALehDX5KHWfE3EPafL9mo1tLkYx_AWvHtFH6KQWaNwDNA5tHcA@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Mar 19, 2021 at 7:37
PM Philip Reames <<a
href="mailto:listmail@philipreames.com"
moz-do-not-send="true">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<p><br>
</p>
<div>On 3/19/21 1:54 PM, Eric Christopher wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Mar 19,
2021 at 2:17 PM Philip Reames via llvm-commits
<<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><br>
Author: Philip Reames<br>
Date: 2021-03-19T11:17:19-07:00<br>
New Revision:
5698537f81a2ecf1166f41cab264b92af670aaa1<br>
<br>
URL: <a
href="https://github.com/llvm/llvm-project/commit/5698537f81a2ecf1166f41cab264b92af670aaa1"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/5698537f81a2ecf1166f41cab264b92af670aaa1</a><br>
DIFF: <a
href="https://github.com/llvm/llvm-project/commit/5698537f81a2ecf1166f41cab264b92af670aaa1.diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/5698537f81a2ecf1166f41cab264b92af670aaa1.diff</a><br>
<br>
LOG: Update basic deref API to account for
possiblity of free [NFC]<br>
<br>
This patch is plumbing to support work towards the
goal outlined in the recent llvm-dev post
"[llvm-dev] RFC: Decomposing deref(N) into
deref(N) + nofree".<br>
<br>
The point of this change is purely to simplify
iteration on other pieces on way to making the
switch. Rebuilding with a change to Value.h is
slow and painful, so I want to get the API change
landed. Once that's done, I plan to more closely
audit each caller, add the inference rules in
their own patch, then post a patch with the
langref changes and test diffs. The value of the
command line flag is that we can exercise the
inference logic in standalone patches without
needing the whole switch ready to go just yet.<br>
<br>
Differential Revision: <a
href="https://reviews.llvm.org/D98908"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://reviews.llvm.org/D98908</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/include/llvm/Analysis/MemoryBuiltins.h<br>
llvm/include/llvm/IR/Value.h<br>
llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
llvm/lib/Analysis/CaptureTracking.cpp<br>
llvm/lib/Analysis/Loads.cpp<br>
llvm/lib/IR/Value.cpp<br>
llvm/lib/Transforms/IPO/AttributorAttributes.cpp<br>
llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git
a/llvm/include/llvm/Analysis/MemoryBuiltins.h
b/llvm/include/llvm/Analysis/MemoryBuiltins.h<br>
index c5428726995e..39ade20df53f 100644<br>
--- a/llvm/include/llvm/Analysis/MemoryBuiltins.h<br>
+++ b/llvm/include/llvm/Analysis/MemoryBuiltins.h<br>
@@ -212,6 +212,10 @@ struct ObjectSizeOpts {<br>
/// object size in Size if successful, and false
otherwise. In this context, by<br>
/// object we mean the region of memory starting
at Ptr to the end of the<br>
/// underlying object pointed to by Ptr.<br>
+///<br>
+/// WARNING: The object size returned is the
allocation size. This does not<br>
+/// imply dereferenceability at site of use since
the object may be freeed in<br>
+/// between.<br>
bool getObjectSize(const Value *Ptr, uint64_t
&Size, const DataLayout &DL,<br>
const TargetLibraryInfo *TLI,
ObjectSizeOpts Opts = {});<br>
<br>
<br>
diff --git a/llvm/include/llvm/IR/Value.h
b/llvm/include/llvm/IR/Value.h<br>
index 5a7e90aeb0f6..e9a9acfd69ba 100644<br>
--- a/llvm/include/llvm/IR/Value.h<br>
+++ b/llvm/include/llvm/IR/Value.h<br>
@@ -743,8 +743,13 @@ class Value {<br>
///<br>
/// If CanBeNull is set by this function the
pointer can either be null or be<br>
/// dereferenceable up to the returned number
of bytes.<br>
+ ///<br>
+ /// IF CanBeFreed is true, the pointer is known
to be dereferenceable at<br>
+ /// point of definition only. Caller must
prove that allocation is not<br>
+ /// deallocated between point of definition and
use.<br>
uint64_t getPointerDereferenceableBytes(const
DataLayout &DL,<br>
- bool
&CanBeNull) const;<br>
+ bool
&CanBeNull,<br>
+ bool
&CanBeFreed) const;<br>
</blockquote>
<div><br>
</div>
<div>FWIW I'm not a fan of this particular pattern -
I find things like:</div>
<div><br>
</div>
<div>foo(dl, false, true)</div>
<div><br>
</div>
<div>to be confusing and a lot less than readable as
a pattern. Can we instead either use multiple
routines, different interfaces, enums, etc?</div>
</div>
</div>
</blockquote>
<p>I think you may have misread the code. You seem to be
thinking these are configuration params, but the actual
code is using them for out params. <br>
</p>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
<div>Fair statement, it was late and I was extremely tired.
Relatedly I'm not a fan of multiple out params passed by
reference. Perhaps this should return a named struct instead
that gives documented return values rather than bools passed
by reference?</div>
</div>
</div>
</blockquote>
Just an update so that you know this hasn't been forgotten... My
current plan is to standardize everything on the canBeFreed()
interface. This essentially lets me back out the original patch you
replied to. I'm holding off on that until a couple of other patches
have landed (mostly for clarity sake), but haven't forgotten this.<br>
<blockquote type="cite"
cite="mid:CALehDX5KHWfE3EPafL9mo1tLkYx_AWvHtFH6KQWaNwDNA5tHcA@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>:)</div>
<div><br>
</div>
<div>-eric</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<div>Thanks!</div>
<div><br>
</div>
<div>-eric</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"> <br>
/// Returns an alignment of the pointer value.<br>
///<br>
<br>
diff --git
a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
b/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
index 11fa4d2893e6..a8c5b9ca80e4 100644<br>
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
@@ -199,9 +199,11 @@ static uint64_t
getMinimalExtentFrom(const Value &V,<br>
// If we have dereferenceability information we
know a lower bound for the<br>
// extent as accesses for a lower offset would
be valid. We need to exclude<br>
// the "or null" part if null is a valid
pointer.<br>
- bool CanBeNull;<br>
- uint64_t DerefBytes =
V.getPointerDereferenceableBytes(DL, CanBeNull);<br>
+ bool CanBeNull, CanBeFreed;<br>
+ uint64_t DerefBytes =<br>
+ V.getPointerDereferenceableBytes(DL,
CanBeNull, CanBeFreed);<br>
DerefBytes = (CanBeNull &&
NullIsValidLoc) ? 0 : DerefBytes;<br>
+ DerefBytes = CanBeFreed ? 0 : DerefBytes;<br>
// If queried with a precise location size, we
assume that location size to be<br>
// accessed, thus valid.<br>
if (LocSize.isPrecise())<br>
<br>
diff --git
a/llvm/lib/Analysis/CaptureTracking.cpp
b/llvm/lib/Analysis/CaptureTracking.cpp<br>
index b2fc6e603f9e..cf5e53b26b5d 100644<br>
--- a/llvm/lib/Analysis/CaptureTracking.cpp<br>
+++ b/llvm/lib/Analysis/CaptureTracking.cpp<br>
@@ -68,8 +68,8 @@ bool
CaptureTracker::isDereferenceableOrNull(Value *O,
const DataLayout &DL) {<br>
if (auto *GEP =
dyn_cast<GetElementPtrInst>(O))<br>
if (GEP->isInBounds())<br>
return true;<br>
- bool CanBeNull;<br>
- return O->getPointerDereferenceableBytes(DL,
CanBeNull);<br>
+ bool CanBeNull, CanBeFreed;<br>
+ return O->getPointerDereferenceableBytes(DL,
CanBeNull, CanBeFreed);<br>
}<br>
<br>
namespace {<br>
<br>
diff --git a/llvm/lib/Analysis/Loads.cpp
b/llvm/lib/Analysis/Loads.cpp<br>
index 88e4c723331a..7279ed59c440 100644<br>
--- a/llvm/lib/Analysis/Loads.cpp<br>
+++ b/llvm/lib/Analysis/Loads.cpp<br>
@@ -67,10 +67,12 @@ static bool
isDereferenceableAndAlignedPointer(<br>
Visited, MaxDepth);<br>
}<br>
<br>
- bool CheckForNonNull = false;<br>
+ bool CheckForNonNull, CheckForFreed;<br>
APInt KnownDerefBytes(Size.getBitWidth(),<br>
-
V->getPointerDereferenceableBytes(DL,
CheckForNonNull));<br>
- if (KnownDerefBytes.getBoolValue() &&
KnownDerefBytes.uge(Size))<br>
+
V->getPointerDereferenceableBytes(DL,
CheckForNonNull,<br>
+
CheckForFreed));<br>
+ if (KnownDerefBytes.getBoolValue() &&
KnownDerefBytes.uge(Size) &&<br>
+ !CheckForFreed)<br>
if (!CheckForNonNull || isKnownNonZero(V, DL,
0, nullptr, CtxI, DT)) {<br>
// As we recursed through GEPs to get here,
we've incrementally checked<br>
// that each step advanced by a multiple of
the alignment. If our base is<br>
<br>
diff --git a/llvm/lib/IR/Value.cpp
b/llvm/lib/IR/Value.cpp<br>
index 92ffae18ae6f..cfb91b55f707 100644<br>
--- a/llvm/lib/IR/Value.cpp<br>
+++ b/llvm/lib/IR/Value.cpp<br>
@@ -38,6 +38,11 @@<br>
<br>
using namespace llvm;<br>
<br>
+static cl::opt<unsigned>
UseDerefAtPointSemantics(<br>
+ "use-dereferenceable-at-point-semantics",
cl::Hidden, cl::init(false),<br>
+ cl::desc("Deref attributes and metadata infer
facts at definition only"));<br>
+<br>
+<br>
static cl::opt<unsigned>
NonGlobalValueMaxNameSize(<br>
"non-global-value-max-name-size", cl::Hidden,
cl::init(1024),<br>
cl::desc("Maximum size for the name of
non-global values."));<br>
@@ -724,11 +729,13 @@
Value::stripInBoundsOffsets(function_ref<void(const
Value *)> Func) const {<br>
}<br>
<br>
uint64_t
Value::getPointerDereferenceableBytes(const
DataLayout &DL,<br>
-
bool &CanBeNull) const {<br>
+
bool &CanBeNull,<br>
+
bool &CanBeFreed) const {<br>
assert(getType()->isPointerTy() &&
"must be pointer");<br>
<br>
uint64_t DerefBytes = 0;<br>
CanBeNull = false;<br>
+ CanBeFreed = UseDerefAtPointSemantics;<br>
if (const Argument *A =
dyn_cast<Argument>(this)) {<br>
DerefBytes = A->getDereferenceableBytes();<br>
if (DerefBytes == 0) {<br>
@@ -783,6 +790,7 @@ uint64_t
Value::getPointerDereferenceableBytes(const
DataLayout &DL,<br>
DerefBytes =<br>
DL.getTypeStoreSize(AI->getAllocatedType()).getKnownMinSize();<br>
CanBeNull = false;<br>
+ CanBeFreed = false;<br>
}<br>
} else if (auto *GV =
dyn_cast<GlobalVariable>(this)) {<br>
if (GV->getValueType()->isSized()
&& !GV->hasExternalWeakLinkage()) {<br>
@@ -790,6 +798,7 @@ uint64_t
Value::getPointerDereferenceableBytes(const
DataLayout &DL,<br>
// CanBeNull flag.<br>
DerefBytes =
DL.getTypeStoreSize(GV->getValueType()).getFixedSize();<br>
CanBeNull = false;<br>
+ CanBeFreed = false;<br>
}<br>
}<br>
return DerefBytes;<br>
<br>
diff --git
a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp<br>
index 21fa11aadea8..fa32a22059ac 100644<br>
---
a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp<br>
+++
b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp<br>
@@ -1750,8 +1750,9 @@ struct AANonNullImpl :
AANonNull {<br>
<br>
AANonNull::initialize(A);<br>
<br>
- bool CanBeNull = true;<br>
- if
(V.getPointerDereferenceableBytes(A.getDataLayout(),
CanBeNull)) {<br>
+ bool CanBeNull, CanBeFreed;<br>
+ if
(V.getPointerDereferenceableBytes(A.getDataLayout(),
CanBeNull,<br>
+
CanBeFreed)) {<br>
if (!CanBeNull) {<br>
indicateOptimisticFixpoint();<br>
return;<br>
@@ -3548,10 +3549,10 @@ struct
AADereferenceableImpl : AADereferenceable {<br>
const IRPosition &IRP =
this->getIRPosition();<br>
NonNullAA =
&A.getAAFor<AANonNull>(*this, IRP,
DepClassTy::NONE);<br>
<br>
- bool CanBeNull;<br>
+ bool CanBeNull, CanBeFreed;<br>
takeKnownDerefBytesMaximum(<br>
IRP.getAssociatedValue().getPointerDereferenceableBytes(<br>
- A.getDataLayout(), CanBeNull));<br>
+ A.getDataLayout(), CanBeNull,
CanBeFreed));<br>
<br>
bool IsFnInterface = IRP.isFnInterfaceKind();<br>
Function *FnScope = IRP.getAnchorScope();<br>
@@ -3661,8 +3662,9 @@ struct
AADereferenceableFloating : AADereferenceableImpl
{<br>
if (!Stripped && this == &AA) {<br>
// Use IR information if we did not strip
anything.<br>
// TODO: track globally.<br>
- bool CanBeNull;<br>
- DerefBytes =
Base->getPointerDereferenceableBytes(DL,
CanBeNull);<br>
+ bool CanBeNull, CanBeFreed;<br>
+ DerefBytes =<br>
+
Base->getPointerDereferenceableBytes(DL,
CanBeNull, CanBeFreed);<br>
T.GlobalState.indicatePessimisticFixpoint();<br>
} else {<br>
const DerefState &DS = AA.getState();<br>
<br>
diff --git
a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
index e6e90b915bb8..ae72123e3f00 100644<br>
---
a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
+++
b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
@@ -2593,8 +2593,8 @@ Instruction
*InstCombinerImpl::visitBitCast(BitCastInst
&CI) {<br>
<br>
// If the source pointer is
dereferenceable, then assume it points to an<br>
// allocated object and apply "inbounds" to
the GEP.<br>
- bool CanBeNull;<br>
- if
(Src->getPointerDereferenceableBytes(DL,
CanBeNull)) {<br>
+ bool CanBeNull, CanBeFreed;<br>
+ if
(Src->getPointerDereferenceableBytes(DL,
CanBeNull, CanBeFreed)) {<br>
// In a non-default address space (not
0), a null pointer can not be<br>
// assumed inbounds, so ignore that case
(dereferenceable_or_null).<br>
// The reason is that 'null' is not
treated <br>
diff erently in these address<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
<a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</blockquote>
</body>
</html>