[llvm] r216818 - Fix AddAliasScopeMetadata to not add scopes when deriving from unknown pointers
Hal Finkel
hfinkel at anl.gov
Sat Aug 30 05:48:34 PDT 2014
Author: hfinkel
Date: Sat Aug 30 07:48:33 2014
New Revision: 216818
URL: http://llvm.org/viewvc/llvm-project?rev=216818&view=rev
Log:
Fix AddAliasScopeMetadata to not add scopes when deriving from unknown pointers
The previous implementation of AddAliasScopeMetadata, which adds noalias
metadata to preserve noalias parameter attribute information when inlining had
a flaw: it would add alias.scope metadata to accesses which might have been
derived from pointers other than noalias function parameters. This was
incorrect because even some access known not to alias with all noalias function
parameters could easily alias with an access derived from some other pointer.
Instead, when deriving from some unknown pointer, we cannot add alias.scope
metadata at all. This fixes a miscompile of the test-suite's tramp3d-v4.
Furthermore, we cannot add alias.scope to functions unless we know they
access only argument-derived pointers (currently, we know this only for
memory intrinsics).
Also, we fix a theoretical problem with using the NoCapture attribute to skip
the capture check. This is incorrect (as explained in the comment added), but
would not matter in any code generated by Clang because we get only inferred
nocapture attributes in Clang-generated IR.
This functionality is not yet enabled by default.
Modified:
llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
llvm/trunk/test/Transforms/Inline/noalias-calls.ll
Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=216818&r1=216817&r2=216818&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Sat Aug 30 07:48:33 2014
@@ -471,17 +471,17 @@ static void AddAliasScopeMetadata(CallSi
else if (const AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(I))
PtrArgs.push_back(RMWI->getPointerOperand());
else if (ImmutableCallSite ICS = ImmutableCallSite(I)) {
- // If we know that the call does not access memory, then we'll still
- // know that about the inlined clone of this call site, and we don't
- // need to add metadata.
+ // If we know that the call does not access memory, then we'll still
+ // know that about the inlined clone of this call site, and we don't
+ // need to add metadata.
if (ICS.doesNotAccessMemory())
continue;
for (ImmutableCallSite::arg_iterator AI = ICS.arg_begin(),
AE = ICS.arg_end(); AI != AE; ++AI)
- // We need to check the underlying objects of all arguments, not just
- // the pointer arguments, because we might be passing pointers as
- // integers, etc.
+ // We need to check the underlying objects of all arguments, not just
+ // the pointer arguments, because we might be passing pointers as
+ // integers, etc.
// FIXME: If we know that the call only accesses pointer arguments,
// then we only need to check the pointer arguments.
PtrArgs.push_back(*AI);
@@ -512,12 +512,23 @@ static void AddAliasScopeMetadata(CallSi
// Figure out if we're derived from anything that is not a noalias
// argument.
- bool CanDeriveViaCapture = false;
- for (const Value *V : ObjSet)
- if (!isIdentifiedFunctionLocal(const_cast<Value*>(V))) {
- CanDeriveViaCapture = true;
- break;
+ bool CanDeriveViaCapture = false, UsesAliasingPtr = false;
+ for (const Value *V : ObjSet) {
+ // Is this value a constant that cannot be derived from any pointer
+ // value (we need to exclude constant expressions, for example, that
+ // are formed from arithmetic on global symbols).
+ bool IsNonPtrConst = isa<ConstantInt>(V) || isa<ConstantFP>(V) ||
+ isa<ConstantPointerNull>(V) ||
+ isa<ConstantDataVector>(V) || isa<UndefValue>(V);
+ if (!IsNonPtrConst &&
+ !isIdentifiedFunctionLocal(const_cast<Value*>(V))) {
+ UsesAliasingPtr = true;
+ if (!isa<Argument>(V)) {
+ CanDeriveViaCapture = true;
+ break;
+ }
}
+ }
// First, we want to figure out all of the sets with which we definitely
// don't alias. Iterate over all noalias set, and add those for which:
@@ -526,7 +537,12 @@ static void AddAliasScopeMetadata(CallSi
// 2. The noalias argument has not yet been captured.
for (const Argument *A : NoAliasArgs) {
if (!ObjSet.count(A) && (!CanDeriveViaCapture ||
- A->hasNoCaptureAttr() ||
+ // It might be tempting to skip the
+ // PointerMayBeCapturedBefore check if
+ // A->hasNoCaptureAttr() is true, but this is
+ // incorrect because nocapture only guarantees
+ // that no copies outlive the function, not
+ // that the value cannot be locally captured.
!PointerMayBeCapturedBefore(A,
/* ReturnCaptures */ false,
/* StoreCaptures */ false, I, &DT)))
@@ -537,22 +553,32 @@ static void AddAliasScopeMetadata(CallSi
NI->setMetadata(LLVMContext::MD_noalias, MDNode::concatenate(
NI->getMetadata(LLVMContext::MD_noalias),
MDNode::get(CalledFunc->getContext(), NoAliases)));
+
// Next, we want to figure out all of the sets to which we might belong.
- // We might below to a set if:
- // 1. The noalias argument is in the set of underlying objects
- // or
- // 2. There is some non-noalias argument in our list and the no-alias
- // argument has been captured.
-
- for (const Argument *A : NoAliasArgs) {
- if (ObjSet.count(A) || (CanDeriveViaCapture &&
- PointerMayBeCapturedBefore(A,
- /* ReturnCaptures */ false,
- /* StoreCaptures */ false,
- I, &DT)))
- Scopes.push_back(NewScopes[A]);
+ // We might belong to a set if the noalias argument is in the set of
+ // underlying objects. If there is some non-noalias argument in our list
+ // of underlying objects, then we cannot add a scope because the fact
+ // that some access does not alias with any set of our noalias arguments
+ // cannot itself guarantee that it does not alias with this access
+ // (because there is some pointer of unknown origin involved and the
+ // other access might also depend on this pointer). We also cannot add
+ // scopes to arbitrary functions unless we know they don't access any
+ // non-parameter pointer-values.
+ bool CanAddScopes = !UsesAliasingPtr;
+ if (CanAddScopes && (isa<CallInst>(I) || isa<InvokeInst>(I))) {
+ // FIXME: We should have a way to access the
+ // IntrReadArgMem/IntrReadWriteArgMem properties of intrinsics, and we
+ // should have a way to determine that for regular functions too. For
+ // now, just do this for the memory intrinsics we understand.
+ CanAddScopes = isa<MemIntrinsic>(I);
}
+ if (CanAddScopes)
+ for (const Argument *A : NoAliasArgs) {
+ if (ObjSet.count(A))
+ Scopes.push_back(NewScopes[A]);
+ }
+
if (!Scopes.empty())
NI->setMetadata(LLVMContext::MD_alias_scope, MDNode::concatenate(
NI->getMetadata(LLVMContext::MD_alias_scope),
Modified: llvm/trunk/test/Transforms/Inline/noalias-calls.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/noalias-calls.ll?rev=216818&r1=216817&r2=216818&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/noalias-calls.ll (original)
+++ llvm/trunk/test/Transforms/Inline/noalias-calls.ll Sat Aug 30 07:48:33 2014
@@ -22,8 +22,8 @@ entry:
; CHECK: define void @foo(i8* nocapture %a, i8* nocapture readonly %c, i8* nocapture %b) #1 {
; CHECK: entry:
-; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 16, i32 16, i1 false) #0, !alias.scope !0, !noalias !3
-; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %b, i8* %c, i64 16, i32 16, i1 false) #0, !alias.scope !3, !noalias !0
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 16, i32 16, i1 false) #0, !noalias !0
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %b, i8* %c, i64 16, i32 16, i1 false) #0, !noalias !3
; CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %c, i64 16, i32 16, i1 false) #0, !alias.scope !5
; CHECK: call void @hey() #0, !noalias !5
; CHECK: ret void
@@ -33,9 +33,9 @@ attributes #0 = { nounwind }
attributes #1 = { nounwind uwtable }
; CHECK: !0 = metadata !{metadata !1}
-; CHECK: !1 = metadata !{metadata !1, metadata !2, metadata !"hello: %a"}
+; CHECK: !1 = metadata !{metadata !1, metadata !2, metadata !"hello: %c"}
; CHECK: !2 = metadata !{metadata !2, metadata !"hello"}
; CHECK: !3 = metadata !{metadata !4}
-; CHECK: !4 = metadata !{metadata !4, metadata !2, metadata !"hello: %c"}
-; CHECK: !5 = metadata !{metadata !1, metadata !4}
+; CHECK: !4 = metadata !{metadata !4, metadata !2, metadata !"hello: %a"}
+; CHECK: !5 = metadata !{metadata !4, metadata !1}
More information about the llvm-commits
mailing list