[llvm] r317217 - Revert "[RS4GC] Strip off invariant.start because memory locations arent invariant"

Anna Thomas via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 2 09:45:52 PDT 2017


Author: annat
Date: Thu Nov  2 09:45:51 2017
New Revision: 317217

URL: http://llvm.org/viewvc/llvm-project?rev=317217&view=rev
Log:
Revert "[RS4GC] Strip off invariant.start because memory locations arent invariant"

This reverts commit r317215, investigating the test failure.

Modified:
    llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
    llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll

Modified: llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp?rev=317217&r1=317216&r2=317217&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Thu Nov  2 09:45:51 2017
@@ -125,10 +125,10 @@ struct RewriteStatepointsForGC : public
       Changed |= runOnFunction(F);
 
     if (Changed) {
-      // stripNonValidData asserts that shouldRewriteStatepointsIn
+      // stripNonValidAttributesAndMetadata asserts that shouldRewriteStatepointsIn
       // returns true for at least one function in the module.  Since at least
       // one function changed, we know that the precondition is satisfied.
-      stripNonValidData(M);
+      stripNonValidAttributesAndMetadata(M);
     }
 
     return Changed;
@@ -146,17 +146,15 @@ struct RewriteStatepointsForGC : public
   /// metadata implying dereferenceability that are no longer valid/correct after
   /// RewriteStatepointsForGC has run. This is because semantically, after
   /// RewriteStatepointsForGC runs, all calls to gc.statepoint "free" the entire
-  /// heap. stripNonValidData (conservatively) restores
+  /// heap. stripNonValidAttributesAndMetadata (conservatively) restores
   /// correctness by erasing all attributes in the module that externally imply
   /// dereferenceability. Similar reasoning also applies to the noalias
   /// attributes and metadata. gc.statepoint can touch the entire heap including
   /// noalias objects.
-  /// Apart from attributes and metadata, we also remove instructions that imply
-  /// constant physical memory: llvm.invariant.start.
-  void stripNonValidData(Module &M);
+  void stripNonValidAttributesAndMetadata(Module &M);
 
-  // Helpers for stripNonValidData
-  void stripNonValidDataFromBody(Function &F);
+  // Helpers for stripNonValidAttributesAndMetadata
+  void stripNonValidAttributesAndMetadataFromBody(Function &F);
   void stripNonValidAttributesFromPrototype(Function &F);
 
   // Certain metadata on instructions are invalid after running RS4GC.
@@ -2387,30 +2385,14 @@ void RewriteStatepointsForGC::stripInval
   I.dropUnknownNonDebugMetadata(ValidMetadataAfterRS4GC);
 }
 
-void RewriteStatepointsForGC::stripNonValidDataFromBody(Function &F) {
+void RewriteStatepointsForGC::stripNonValidAttributesAndMetadataFromBody(Function &F) {
   if (F.empty())
     return;
 
   LLVMContext &Ctx = F.getContext();
   MDBuilder Builder(Ctx);
 
-  // Set of invariantstart instructions that we need to remove.
-  // Use this to avoid invalidating the instruction iterator.
-  SmallVector<IntrinsicInst*, 12> InvariantStartInstructions;
-
   for (Instruction &I : instructions(F)) {
-    // invariant.start on memory location implies that the referenced memory
-    // location is constant and unchanging. This is no longer true after
-    // RewriteStatepointsForGC runs because there can be calls to gc.statepoint
-    // which frees the entire heap and the presence of invariant.start allows
-    // the optimizer to sink the load of a memory location past a statepoint,
-    // which is incorrect.
-    if (auto *II = dyn_cast<IntrinsicInst>(&I))
-      if (II->getIntrinsicID() == Intrinsic::invariant_start) {
-        InvariantStartInstructions.push_back(II);
-        continue;
-      }
-
     if (const MDNode *MD = I.getMetadata(LLVMContext::MD_tbaa)) {
       assert(MD->getNumOperands() < 5 && "unrecognized metadata shape!");
       bool IsImmutableTBAA =
@@ -2440,18 +2422,6 @@ void RewriteStatepointsForGC::stripNonVa
         RemoveNonValidAttrAtIndex(Ctx, CS, AttributeList::ReturnIndex);
     }
   }
-
-  // Delete the invariant.start instructions and any corresponding uses that
-  // don't have further uses, for example invariant.end.
-  for (auto *II : InvariantStartInstructions) {
-    for (auto *U : II->users())
-      if (auto *I = dyn_cast<Instruction>(U))
-       if (U->hasNUses(0))
-         I->eraseFromParent();
-    // We cannot just delete the remaining uses of II, so we RAUW undef.
-    II->replaceAllUsesWith(UndefValue::get(II->getType()));
-    II->eraseFromParent();
-  }
 }
 
 /// Returns true if this function should be rewritten by this pass.  The main
@@ -2468,7 +2438,7 @@ static bool shouldRewriteStatepointsIn(F
     return false;
 }
 
-void RewriteStatepointsForGC::stripNonValidData(Module &M) {
+void RewriteStatepointsForGC::stripNonValidAttributesAndMetadata(Module &M) {
 #ifndef NDEBUG
   assert(llvm::any_of(M, shouldRewriteStatepointsIn) && "precondition!");
 #endif
@@ -2477,7 +2447,7 @@ void RewriteStatepointsForGC::stripNonVa
     stripNonValidAttributesFromPrototype(F);
 
   for (Function &F : M)
-    stripNonValidDataFromBody(F);
+    stripNonValidAttributesAndMetadataFromBody(F);
 }
 
 bool RewriteStatepointsForGC::runOnFunction(Function &F) {

Modified: llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll?rev=317217&r1=317216&r2=317217&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll (original)
+++ llvm/trunk/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll Thu Nov  2 09:45:51 2017
@@ -75,59 +75,6 @@ define void @test_dereferenceable(i32 ad
   ret void
 }
 
-; invariant.start allows us to sink the load past the baz statepoint call into taken block, which is
-; incorrect. remove the invariant.start and RAUW undef.
-define void @test_inv_start(i1 %cond, i32 addrspace(1)* addrspace(1)* %p, i32 %x, i32 addrspace(1)* %q) gc "statepoint-example" {
-; CHECK-LABEL: test_inv_start
-; CHECK-NOT: invariant.start
-; CHECK: gc.statepoint
-  %v1 = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(1)* %p
-  %invst = call {}* @llvm.invariant.start.p1i32(i64 1, i32 addrspace(1)* %v1)
-  %v2 = load i32, i32 addrspace(1)* %v1
-  call void @baz(i32 %x)
-  br i1 %cond, label %taken, label %untaken
-
-; CHECK-LABEL: taken:
-; CHECK-NOT: llvm.invariant.end
-taken:
-  store i32 %v2, i32 addrspace(1)* %q, align 16
-  call void @llvm.invariant.end.p1i32({}* %invst, i64 4, i32 addrspace(1)* %v1)
-  ret void
-
-; CHECK-LABEL: untaken:
-; CHECK: gc.statepoint
-untaken:
-  %foo = call i32 @escaping.invariant.start({}* %invst)
-  call void @dummy(i32 %foo)
-  ret void
-}
-
-; invariant.start and end is removed. No other uses.
-define void @test_inv_start2(i1 %cond, i32 addrspace(1)* addrspace(1)* %p, i32 %x, i32 addrspace(1)* %q) gc "statepoint-example" {
-; CHECK-LABEL: test_inv_start2
-; CHECK-NOT: invariant.start
-; CHECK: gc.statepoint
-  %v1 = load i32 addrspace(1)*, i32 addrspace(1)* addrspace(1)* %p
-  %invst = call {}* @llvm.invariant.start.p1i32(i64 1, i32 addrspace(1)* %v1)
-  %v2 = load i32, i32 addrspace(1)* %v1
-  call void @baz(i32 %x)
-  br i1 %cond, label %taken, label %untaken
-
-; CHECK-LABEL: taken:
-; CHECK-NOT: llvm.invariant.end
-taken:
-  store i32 %v2, i32 addrspace(1)* %q, align 16
-  call void @llvm.invariant.end.p1i32({}* %invst, i64 4, i32 addrspace(1)* %v1)
-  ret void
-
-; CHECK-LABEL: untaken:
-untaken:
-  ret void
-}
-declare {}* @llvm.invariant.start.p1i32(i64, i32 addrspace(1)*  nocapture) nounwind readonly
-declare void @llvm.invariant.end.p1i32({}*, i64, i32 addrspace(1)* nocapture) nounwind
-declare i32 @escaping.invariant.start({}*) nounwind
-declare void @dummy(i32)
 declare token @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64, i32, void (i32)*, i32, i32, ...)
 
 ; Function Attrs: nounwind readonly




More information about the llvm-commits mailing list