[llvm] r353562 - [CodeExtractor] Restore outputs after creating exit stubs
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 8 12:48:04 PST 2019
Author: vedantk
Date: Fri Feb 8 12:48:04 2019
New Revision: 353562
URL: http://llvm.org/viewvc/llvm-project?rev=353562&view=rev
Log:
[CodeExtractor] Restore outputs after creating exit stubs
When CodeExtractor saves the result of InvokeInst at the first insertion
point of the 'normal destination' basic block, this block can be omitted
in the outlined region, so store is placed outside of the function. The
suggested solution is to process saving outputs after creating exit
stubs for new function, and stores will be placed in that blocks before
return in this case.
Patch by Sergei Kachkov!
Fixes llvm.org/PR40455.
Differential Revision: https://reviews.llvm.org/D57919
Modified:
llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp
llvm/trunk/unittests/Transforms/Utils/CodeExtractorTest.cpp
Modified: llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp?rev=353562&r1=353561&r2=353562&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp Fri Feb 8 12:48:04 2019
@@ -1046,7 +1046,6 @@ CallInst *CodeExtractor::emitCallAndSwit
std::advance(OutputArgBegin, inputs.size());
// Reload the outputs passed in by reference.
- Function::arg_iterator OAI = OutputArgBegin;
for (unsigned i = 0, e = outputs.size(); i != e; ++i) {
Value *Output = nullptr;
if (AggregateArgs) {
@@ -1070,40 +1069,6 @@ CallInst *CodeExtractor::emitCallAndSwit
if (!Blocks.count(inst->getParent()))
inst->replaceUsesOfWith(outputs[i], load);
}
-
- // Store to argument right after the definition of output value.
- auto *OutI = dyn_cast<Instruction>(outputs[i]);
- if (!OutI)
- continue;
-
- // Find proper insertion point.
- BasicBlock::iterator InsertPt;
- // In case OutI is an invoke, we insert the store at the beginning in the
- // 'normal destination' BB. Otherwise we insert the store right after OutI.
- if (auto *InvokeI = dyn_cast<InvokeInst>(OutI))
- InsertPt = InvokeI->getNormalDest()->getFirstInsertionPt();
- else if (auto *Phi = dyn_cast<PHINode>(OutI))
- InsertPt = Phi->getParent()->getFirstInsertionPt();
- else
- InsertPt = std::next(OutI->getIterator());
-
- assert(OAI != newFunction->arg_end() &&
- "Number of output arguments should match "
- "the amount of defined values");
- if (AggregateArgs) {
- Value *Idx[2];
- Idx[0] = Constant::getNullValue(Type::getInt32Ty(Context));
- Idx[1] = ConstantInt::get(Type::getInt32Ty(Context), FirstOut + i);
- GetElementPtrInst *GEP = GetElementPtrInst::Create(
- StructArgTy, &*OAI, Idx, "gep_" + outputs[i]->getName(), &*InsertPt);
- new StoreInst(outputs[i], GEP, &*InsertPt);
- // Since there should be only one struct argument aggregating
- // all the output values, we shouldn't increment OAI, which always
- // points to the struct argument, in this case.
- } else {
- new StoreInst(outputs[i], &*OAI, &*InsertPt);
- ++OAI;
- }
}
// Now we can emit a switch statement using the call as a value.
@@ -1159,6 +1124,50 @@ CallInst *CodeExtractor::emitCallAndSwit
}
}
+ // Store the arguments right after the definition of output value.
+ // This should be proceeded after creating exit stubs to be ensure that invoke
+ // result restore will be placed in the outlined function.
+ Function::arg_iterator OAI = OutputArgBegin;
+ for (unsigned i = 0, e = outputs.size(); i != e; ++i) {
+ auto *OutI = dyn_cast<Instruction>(outputs[i]);
+ if (!OutI)
+ continue;
+
+ // Find proper insertion point.
+ BasicBlock::iterator InsertPt;
+ // In case OutI is an invoke, we insert the store at the beginning in the
+ // 'normal destination' BB. Otherwise we insert the store right after OutI.
+ if (auto *InvokeI = dyn_cast<InvokeInst>(OutI))
+ InsertPt = InvokeI->getNormalDest()->getFirstInsertionPt();
+ else if (auto *Phi = dyn_cast<PHINode>(OutI))
+ InsertPt = Phi->getParent()->getFirstInsertionPt();
+ else
+ InsertPt = std::next(OutI->getIterator());
+
+ Instruction *InsertBefore = &*InsertPt;
+ assert((InsertBefore->getFunction() == newFunction ||
+ Blocks.count(InsertBefore->getParent())) &&
+ "InsertPt should be in new function");
+ assert(OAI != newFunction->arg_end() &&
+ "Number of output arguments should match "
+ "the amount of defined values");
+ if (AggregateArgs) {
+ Value *Idx[2];
+ Idx[0] = Constant::getNullValue(Type::getInt32Ty(Context));
+ Idx[1] = ConstantInt::get(Type::getInt32Ty(Context), FirstOut + i);
+ GetElementPtrInst *GEP = GetElementPtrInst::Create(
+ StructArgTy, &*OAI, Idx, "gep_" + outputs[i]->getName(),
+ InsertBefore);
+ new StoreInst(outputs[i], GEP, InsertBefore);
+ // Since there should be only one struct argument aggregating
+ // all the output values, we shouldn't increment OAI, which always
+ // points to the struct argument, in this case.
+ } else {
+ new StoreInst(outputs[i], &*OAI, InsertBefore);
+ ++OAI;
+ }
+ }
+
// Now that we've done the deed, simplify the switch instruction.
Type *OldFnRetTy = TheSwitch->getParent()->getParent()->getReturnType();
switch (NumExitBlocks) {
Modified: llvm/trunk/unittests/Transforms/Utils/CodeExtractorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/CodeExtractorTest.cpp?rev=353562&r1=353561&r2=353562&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/CodeExtractorTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/CodeExtractorTest.cpp Fri Feb 8 12:48:04 2019
@@ -58,8 +58,7 @@ TEST(CodeExtractor, ExitStub) {
getBlockByName(Func, "body1"),
getBlockByName(Func, "body2") };
- DominatorTree DT(*Func);
- CodeExtractor CE(Candidates, &DT);
+ CodeExtractor CE(Candidates);
EXPECT_TRUE(CE.isEligible());
Function *Outlined = CE.extractCodeRegion();
@@ -109,8 +108,7 @@ TEST(CodeExtractor, ExitPHIOnePredFromRe
getBlockByName(Func, "extracted2")
};
- DominatorTree DT(*Func);
- CodeExtractor CE(ExtractedBlocks, &DT);
+ CodeExtractor CE(ExtractedBlocks);
EXPECT_TRUE(CE.isEligible());
Function *Outlined = CE.extractCodeRegion();
@@ -184,8 +182,7 @@ TEST(CodeExtractor, StoreOutputInvokeRes
getBlockByName(Func, "lpad2")
};
- DominatorTree DT(*Func);
- CodeExtractor CE(ExtractedBlocks, &DT);
+ CodeExtractor CE(ExtractedBlocks);
EXPECT_TRUE(CE.isEligible());
Function *Outlined = CE.extractCodeRegion();
@@ -194,4 +191,38 @@ TEST(CodeExtractor, StoreOutputInvokeRes
EXPECT_FALSE(verifyFunction(*Func, &errs()));
}
+TEST(CodeExtractor, StoreOutputInvokeResultInExitStub) {
+ LLVMContext Ctx;
+ SMDiagnostic Err;
+ std::unique_ptr<Module> M(parseAssemblyString(R"invalid(
+ declare i32 @bar()
+
+ define i32 @foo() personality i8* null {
+ entry:
+ %0 = invoke i32 @bar() to label %exit unwind label %lpad
+
+ exit:
+ ret i32 %0
+
+ lpad:
+ %1 = landingpad { i8*, i32 }
+ cleanup
+ resume { i8*, i32 } %1
+ }
+ )invalid",
+ Err, Ctx));
+
+ Function *Func = M->getFunction("foo");
+ SmallVector<BasicBlock *, 1> Blocks{ getBlockByName(Func, "entry"),
+ getBlockByName(Func, "lpad") };
+
+ CodeExtractor CE(Blocks);
+ EXPECT_TRUE(CE.isEligible());
+
+ Function *Outlined = CE.extractCodeRegion();
+ EXPECT_TRUE(Outlined);
+ EXPECT_FALSE(verifyFunction(*Outlined));
+ EXPECT_FALSE(verifyFunction(*Func));
+}
+
} // end anonymous namespace
More information about the llvm-commits
mailing list