From: Anna Thomas Date: Thu, 2 Nov 2017 16:45:51 +0000 (+0000) Subject: Revert "[RS4GC] Strip off invariant.start because memory locations arent invariant" X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ebe429d99f539c876082597e6790b51ef28f50b0;p=platform%2Fupstream%2Fllvm.git Revert "[RS4GC] Strip off invariant.start because memory locations arent invariant" This reverts commit r317215, investigating the test failure. llvm-svn: 317217 --- diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 9a06482..1ca77cf 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) { - // 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 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. 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::stripInvalidMetadataFromInstruction(Instruction &I 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 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(&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::stripNonValidDataFromBody(Function &F) { 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(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(Function &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::stripNonValidData(Module &M) { stripNonValidAttributesFromPrototype(F); for (Function &F : M) - stripNonValidDataFromBody(F); + stripNonValidAttributesAndMetadataFromBody(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 4f3ab6a..105afa9 100644 --- a/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll +++ b/llvm/test/Transforms/RewriteStatepointsForGC/drop-invalid-metadata.ll @@ -75,59 +75,6 @@ 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