Skip to content

Commit 334a1cd

Browse files
authored
[SandboxIR] createFunction() should always create a function (#124665)
This patch removes the assertion that checks for an existing function. If one exists it will remove it and create a new one. This helps remove a crash when a function declaration object already exists and we are about to create a SandboxIR object for the definition.
1 parent fa9ac62 commit 334a1cd

File tree

3 files changed

+28
-2
lines changed

3 files changed

+28
-2
lines changed

llvm/lib/SandboxIR/Context.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,12 +628,15 @@ Module *Context::getOrCreateModule(llvm::Module *LLVMM) {
628628
}
629629

630630
Function *Context::createFunction(llvm::Function *F) {
631-
assert(getValue(F) == nullptr && "Already exists!");
632631
// Create the module if needed before we create the new sandboxir::Function.
633632
// Note: this won't fully populate the module. The only globals that will be
634633
// available will be the ones being used within the function.
635634
getOrCreateModule(F->getParent());
636635

636+
// There may be a function declaration already defined. Regardless destroy it.
637+
if (Function *ExistingF = cast_or_null<Function>(getValue(F)))
638+
detach(ExistingF);
639+
637640
auto NewFPtr = std::unique_ptr<Function>(new Function(F, *this));
638641
auto *SBF = cast<Function>(registerValue(std::move(NewFPtr)));
639642
// Create arguments.

llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,8 @@ bool SandboxVectorizerPass::runImpl(Function &LLVMF) {
8787
// Create SandboxIR for LLVMF and run BottomUpVec on it.
8888
sandboxir::Function &F = *Ctx->createFunction(&LLVMF);
8989
sandboxir::Analyses A(*AA, *SE, *TTI);
90-
return FPM.runOnFunction(F, A);
90+
bool Change = FPM.runOnFunction(F, A);
91+
// TODO: This is a function pass, so we won't be needing the function-level
92+
// Sandbox IR objects in the future. So we should clear them.
93+
return Change;
9194
}

llvm/unittests/SandboxIR/SandboxIRTest.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6080,3 +6080,23 @@ TEST_F(SandboxIRTest, InstructionCallbacks) {
60806080
EXPECT_THAT(Removed, testing::IsEmpty());
60816081
EXPECT_THAT(Moved, testing::IsEmpty());
60826082
}
6083+
6084+
TEST_F(SandboxIRTest, FunctionObjectAlreadyExists) {
6085+
parseIR(C, R"IR(
6086+
define void @foo() {
6087+
call void @bar()
6088+
ret void
6089+
}
6090+
define void @bar() {
6091+
ret void
6092+
}
6093+
)IR");
6094+
Function &LLVMFoo = *M->getFunction("foo");
6095+
Function &LLVMBar = *M->getFunction("bar");
6096+
sandboxir::Context Ctx(C);
6097+
// This will create a Function object for @bar().
6098+
Ctx.createFunction(&LLVMFoo);
6099+
EXPECT_NE(Ctx.getValue(&LLVMBar), nullptr);
6100+
// This should not crash, even though there is already a value for LLVMBar.
6101+
Ctx.createFunction(&LLVMBar);
6102+
}

0 commit comments

Comments
 (0)