diff options
| -rw-r--r-- | llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp | 48 | ||||
| -rw-r--r-- | llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll | 53 | 
2 files changed, 92 insertions, 9 deletions
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 1ca77cfec32..9a064829dee 100644 --- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -125,10 +125,10 @@ struct RewriteStatepointsForGC : public ModulePass {        Changed |= runOnFunction(F);      if (Changed) { -      // stripNonValidAttributesAndMetadata asserts that shouldRewriteStatepointsIn +      // stripNonValidData 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. -      stripNonValidAttributesAndMetadata(M); +      stripNonValidData(M);      }      return Changed; @@ -146,15 +146,17 @@ struct RewriteStatepointsForGC : public ModulePass {    /// 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. stripNonValidAttributesAndMetadata (conservatively) restores +  /// heap. stripNonValidData (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. -  void stripNonValidAttributesAndMetadata(Module &M); +  /// Apart from attributes and metadata, we also remove instructions that imply +  /// constant physical memory: llvm.invariant.start. +  void stripNonValidData(Module &M); -  // Helpers for stripNonValidAttributesAndMetadata -  void stripNonValidAttributesAndMetadataFromBody(Function &F); +  // Helpers for stripNonValidData +  void stripNonValidDataFromBody(Function &F);    void stripNonValidAttributesFromPrototype(Function &F);    // Certain metadata on instructions are invalid after running RS4GC. @@ -2385,14 +2387,30 @@ void RewriteStatepointsForGC::stripInvalidMetadataFromInstruction(Instruction &I    I.dropUnknownNonDebugMetadata(ValidMetadataAfterRS4GC);  } -void RewriteStatepointsForGC::stripNonValidAttributesAndMetadataFromBody(Function &F) { +void RewriteStatepointsForGC::stripNonValidDataFromBody(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 = @@ -2422,6 +2440,18 @@ void RewriteStatepointsForGC::stripNonValidAttributesAndMetadataFromBody(Functio          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 @@ -2438,7 +2468,7 @@ static bool shouldRewriteStatepointsIn(Function &F) {      return false;  } -void RewriteStatepointsForGC::stripNonValidAttributesAndMetadata(Module &M) { +void RewriteStatepointsForGC::stripNonValidData(Module &M) {  #ifndef NDEBUG    assert(llvm::any_of(M, shouldRewriteStatepointsIn) && "precondition!");  #endif @@ -2447,7 +2477,7 @@ void RewriteStatepointsForGC::stripNonValidAttributesAndMetadata(Module &M) {      stripNonValidAttributesFromPrototype(F);    for (Function &F : M) -    stripNonValidAttributesAndMetadataFromBody(F); +    stripNonValidDataFromBody(F);  }  bool RewriteStatepointsForGC::runOnFunction(Function &F) { diff --git a/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll b/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll index 105afa9def5..4f3ab6a4beb 100644 --- a/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll +++ b/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll @@ -75,6 +75,59 @@ define void @test_dereferenceable(i32 addrspace(1)* addrspace(1)* %p, i32 %x, i3    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  | 

