[llvm-branch-commits] [llvm-branch] r277639 - Merging r277625:
Hans Wennborg via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Aug 3 12:52:03 PDT 2016
Author: hans
Date: Wed Aug 3 14:52:03 2016
New Revision: 277639
URL: http://llvm.org/viewvc/llvm-project?rev=277639&view=rev
Log:
Merging r277625:
------------------------------------------------------------------------
r277625 | dexonsmith | 2016-08-03 11:19:43 -0700 (Wed, 03 Aug 2016) | 42 lines
IR: Drop uniquing when an MDNode Value operand is deleted
This is a fix for PR28697.
An MDNode can indirectly refer to a GlobalValue, through a
ConstantAsMetadata. When the GlobalValue is deleted, the MDNode operand
is reset to `nullptr`. If the node is uniqued, this can lead to a
hard-to-detect cache invalidation in a Metadata map that's shared across
an LLVMContext.
Consider:
1. A map from Metadata* to `T` called RemappedMDs.
2. A node that references a global variable, `!{i1* @GV}`.
3. Insert `!{i1* @GV} -> SomeT` in the map.
4. Delete `@GV`, leaving behind `!{null} -> SomeT`.
Looking up the generic and uninteresting `!{null}` gives you `SomeT`,
which is likely related to `@GV`. Worse, `SomeT`'s lifetime may be tied
to the deleted `@GV`.
This occurs in practice in the shared ValueMap used since r266579 in the
IRMover. Other code that handles more than one Module (with different
lifetimes) in the same LLVMContext could hit it too.
The fix here is a partial revert of r225223: in the rare case that an
MDNode operand is a ConstantAsMetadata (i.e., wrapping a node from the
Value hierarchy), drop uniquing if it gets replaced with `nullptr`.
This changes step #4 above to leave behind `distinct !{null} -> SomeT`,
which can't be confused with the generic `!{null}`.
In theory, this can cause some churn in the LLVMContext's MDNode
uniquing map when Values are being deleted. However:
- The number of GlobalValues referenced from uniqued MDNodes is
expected to be quite small. E.g., the debug info metadata schema
only references GlobalValues from distinct nodes.
- Other Constants have the lifetime of the LLVMContext, whose teardown
is careful to drop references before deleting the constants.
As a result, I don't expect a compile time regression from this change.
------------------------------------------------------------------------
Added:
llvm/branches/release_39/test/Linker/Inputs/metadata-with-global-value-operand.ll
- copied unchanged from r277625, llvm/trunk/test/Linker/Inputs/metadata-with-global-value-operand.ll
llvm/branches/release_39/test/Linker/metadata-with-global-value-operand.ll
- copied unchanged from r277625, llvm/trunk/test/Linker/metadata-with-global-value-operand.ll
Modified:
llvm/branches/release_39/ (props changed)
llvm/branches/release_39/lib/IR/Metadata.cpp
llvm/branches/release_39/test/Transforms/GlobalOpt/metadata.ll
llvm/branches/release_39/unittests/IR/MetadataTest.cpp
Propchange: llvm/branches/release_39/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Aug 3 14:52:03 2016
@@ -1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,275868-275870,275879,275898,275928,275935,275946,275978,275981,276015,276077,276109,276119,276181,276209,276236-276237,276358,276364,276368,276389,276435,276438,276479,276510,276648,276740,276956,276980,277114,277135,277371,277500,277504
+/llvm/trunk:155241,275868-275870,275879,275898,275928,275935,275946,275978,275981,276015,276077,276109,276119,276181,276209,276236-276237,276358,276364,276368,276389,276435,276438,276479,276510,276648,276740,276956,276980,277114,277135,277371,277500,277504,277625
Modified: llvm/branches/release_39/lib/IR/Metadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_39/lib/IR/Metadata.cpp?rev=277639&r1=277638&r2=277639&view=diff
==============================================================================
--- llvm/branches/release_39/lib/IR/Metadata.cpp (original)
+++ llvm/branches/release_39/lib/IR/Metadata.cpp Wed Aug 3 14:52:03 2016
@@ -675,8 +675,8 @@ void MDNode::handleChangedOperand(void *
Metadata *Old = getOperand(Op);
setOperand(Op, New);
- // Drop uniquing for self-reference cycles.
- if (New == this) {
+ // Drop uniquing for self-reference cycles and deleted constants.
+ if (New == this || (!New && Old && isa<ConstantAsMetadata>(Old))) {
if (!isResolved())
resolve();
storeDistinctInContext();
Modified: llvm/branches/release_39/test/Transforms/GlobalOpt/metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_39/test/Transforms/GlobalOpt/metadata.ll?rev=277639&r1=277638&r2=277639&view=diff
==============================================================================
--- llvm/branches/release_39/test/Transforms/GlobalOpt/metadata.ll (original)
+++ llvm/branches/release_39/test/Transforms/GlobalOpt/metadata.ll Wed Aug 3 14:52:03 2016
@@ -28,5 +28,5 @@ declare void @llvm.foo(metadata, metadat
; CHECK: !named = !{![[NULL:[0-9]+]]}
!0 = !{i8*** @G}
-; CHECK-DAG: ![[NULL]] = !{null}
+; CHECK-DAG: ![[NULL]] = distinct !{null}
; CHECK-DAG: ![[EMPTY]] = !{}
Modified: llvm/branches/release_39/unittests/IR/MetadataTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_39/unittests/IR/MetadataTest.cpp?rev=277639&r1=277638&r2=277639&view=diff
==============================================================================
--- llvm/branches/release_39/unittests/IR/MetadataTest.cpp (original)
+++ llvm/branches/release_39/unittests/IR/MetadataTest.cpp Wed Aug 3 14:52:03 2016
@@ -449,6 +449,40 @@ TEST_F(MDNodeTest, DistinctOnUniquingCol
EXPECT_FALSE(Wrapped1->isDistinct());
}
+TEST_F(MDNodeTest, UniquedOnDeletedOperand) {
+ // temp !{}
+ TempMDTuple T = MDTuple::getTemporary(Context, None);
+
+ // !{temp !{}}
+ Metadata *Ops[] = {T.get()};
+ MDTuple *N = MDTuple::get(Context, Ops);
+
+ // !{temp !{}} => !{null}
+ T.reset();
+ ASSERT_TRUE(N->isUniqued());
+ Metadata *NullOps[] = {nullptr};
+ ASSERT_EQ(N, MDTuple::get(Context, NullOps));
+}
+
+TEST_F(MDNodeTest, DistinctOnDeletedValueOperand) {
+ // i1* @GV
+ Type *Ty = Type::getInt1PtrTy(Context);
+ std::unique_ptr<GlobalVariable> GV(
+ new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
+ ConstantAsMetadata *Op = ConstantAsMetadata::get(GV.get());
+
+ // !{i1* @GV}
+ Metadata *Ops[] = {Op};
+ MDTuple *N = MDTuple::get(Context, Ops);
+
+ // !{i1* @GV} => !{null}
+ GV.reset();
+ ASSERT_TRUE(N->isDistinct());
+ ASSERT_EQ(nullptr, N->getOperand(0));
+ Metadata *NullOps[] = {nullptr};
+ ASSERT_NE(N, MDTuple::get(Context, NullOps));
+}
+
TEST_F(MDNodeTest, getDistinct) {
// !{}
MDNode *Empty = MDNode::get(Context, None);
@@ -669,7 +703,7 @@ TEST_F(MDNodeTest, replaceWithUniquedRes
EXPECT_TRUE(N->isResolved());
}
-TEST_F(MDNodeTest, replaceWithUniquedChangingOperand) {
+TEST_F(MDNodeTest, replaceWithUniquedDeletedOperand) {
// i1* @GV
Type *Ty = Type::getInt1PtrTy(Context);
std::unique_ptr<GlobalVariable> GV(
@@ -686,8 +720,33 @@ TEST_F(MDNodeTest, replaceWithUniquedCha
// !{i1* @GV} => !{null}
GV.reset();
- ASSERT_TRUE(N->isUniqued());
+ ASSERT_TRUE(N->isDistinct());
+ ASSERT_EQ(nullptr, N->getOperand(0));
Metadata *NullOps[] = {nullptr};
+ ASSERT_NE(N, MDTuple::get(Context, NullOps));
+}
+
+TEST_F(MDNodeTest, replaceWithUniquedChangedOperand) {
+ // i1* @GV
+ Type *Ty = Type::getInt1PtrTy(Context);
+ std::unique_ptr<GlobalVariable> GV(
+ new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
+ ConstantAsMetadata *Op = ConstantAsMetadata::get(GV.get());
+
+ // temp !{i1* @GV}
+ Metadata *Ops[] = {Op};
+ MDTuple *N = MDTuple::getTemporary(Context, Ops).release();
+
+ // temp !{i1* @GV} => !{i1* @GV}
+ ASSERT_EQ(N, MDNode::replaceWithUniqued(TempMDTuple(N)));
+ ASSERT_TRUE(N->isUniqued());
+
+ // !{i1* @GV} => !{i1* @GV2}
+ std::unique_ptr<GlobalVariable> GV2(
+ new GlobalVariable(Ty, false, GlobalValue::ExternalLinkage));
+ GV->replaceAllUsesWith(GV2.get());
+ ASSERT_TRUE(N->isUniqued());
+ Metadata *NullOps[] = {ConstantAsMetadata::get(GV2.get())};
ASSERT_EQ(N, MDTuple::get(Context, NullOps));
}
More information about the llvm-branch-commits
mailing list