bmoody created this revision.
bmoody added reviewers: craig.topper, dberris, rjmccall.
Herald added a subscriber: cfe-commits.
This patch adds a check at the end of CodeGenFunction::FinishFunction to delete
the return block if it isn't reachable.
Without this patch applied the code generated for the test cases is:
define void @f1() {
entry:
call void @abort()
unreachable
return: ; No predecessors!
ret void
}
define i8* @f2() {
entry:
%retval = alloca i8*, align 8
call void @abort()
unreachable
return: ; No predecessors!
%0 = load i8*, i8** %retval, align 8
ret i8* %0
}
This sort-of addresses the FIXME in CodeGenFunction::EmitReturnBlock:
// FIXME: We are at an unreachable point, there is no reason to emit the block
// unless it has uses. However, we still need a place to put the debug
// region.end for now.
I'm not sure if this FIXME still applies with this patch - it's not clear if
the intent is to avoid generating the unreachable code in the first place,
rather than generating it and then deleting it. Seems like it would complicate
the rest of the codegen to avoid generating it in the first place.
Repository:
rC Clang
https://reviews.llvm.org/D57072
Files:
lib/CodeGen/CGCall.cpp
lib/CodeGen/CodeGenFunction.cpp
test/CodeGen/unreachable-ret.c
test/OpenMP/parallel_reduction_codegen.cpp
Index: test/OpenMP/parallel_reduction_codegen.cpp
===================================================================
--- test/OpenMP/parallel_reduction_codegen.cpp
+++ test/OpenMP/parallel_reduction_codegen.cpp
@@ -622,7 +622,7 @@
// CHECK-NOT: call i32 @__kmpc_reduce
-// CHECK: ret void
+// CHECK: }
// CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
// CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],
Index: test/CodeGen/unreachable-ret.c
===================================================================
--- /dev/null
+++ test/CodeGen/unreachable-ret.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+ abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @abort()
+// CHECK-NEXT: unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+ abort();
+ return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @abort()
+// CHECK-NEXT: unreachable
+// CHECK-NEXT: }
+
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -255,6 +255,7 @@
if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
delete ReturnBlock.getBlock();
+ ReturnBlock = JumpDest();
} else
EmitBlock(ReturnBlock.getBlock());
return llvm::DebugLoc();
@@ -274,6 +275,7 @@
Builder.SetInsertPoint(BI->getParent());
BI->eraseFromParent();
delete ReturnBlock.getBlock();
+ ReturnBlock = JumpDest();
return Loc;
}
}
@@ -448,6 +450,19 @@
// 5. Width of vector aguments and return types for functions called by this
// function.
CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+ // If we generated an unreachable return block, delete it now.
+ if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+ Builder.ClearInsertionPoint();
+ ReturnBlock.getBlock()->eraseFromParent();
+ }
+ if (ReturnValue.isValid()) {
+ auto *RetAlloca = dyn_cast<llvm::AllocaInst>(ReturnValue.getPointer());
+ if (RetAlloca && RetAlloca->use_empty()) {
+ RetAlloca->eraseFromParent();
+ ReturnValue = Address::invalid();
+ }
+ }
}
/// ShouldInstrumentFunction - Return true if the current function should be
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -2896,15 +2896,6 @@
RV = SI->getValueOperand();
SI->eraseFromParent();
- // If that was the only use of the return value, nuke it as well now.
- auto returnValueInst = ReturnValue.getPointer();
- if (returnValueInst->use_empty()) {
- if (auto alloca = dyn_cast<llvm::AllocaInst>(returnValueInst)) {
- alloca->eraseFromParent();
- ReturnValue = Address::invalid();
- }
- }
-
// Otherwise, we have to do a simple load.
} else {
RV = Builder.CreateLoad(ReturnValue);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
