danielaustin updated this revision to Diff 43459.
danielaustin added a comment.
Test case added which looks for the existence of multiple trap basic blocks in
generated LLVM-IR with optimization turned on. Flag renamed
(-fmerge-traps/-fno-merge-traps) and placed in a more sensible location
(CodeGenOptions.def). Trap inlining logic changed to work with most recent
version of LLVM (as opposed to the one packaged with AOSP). Without turning
optimization off in the function containing the trap, there were cases that the
abort calls were merged to the end of the function. Turning off inlining and
optimization for a function which is not to merge traps appears to be working,
but further analysis is in process.
This version results in slightly bigger code (~10% per function) than the
original version, but has the benefit of not breaking basic blocks.
Repository:
rL LLVM
http://reviews.llvm.org/D15208
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
include/clang/Frontend/CodeGenOptions.h
lib/CodeGen/CGExpr.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/merge-traps.c
Index: test/CodeGen/merge-traps.c
===================================================================
--- test/CodeGen/merge-traps.c
+++ test/CodeGen/merge-traps.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -emit-llvm -o - -fsanitize=unsigned-integer-overflow -ftrapv -fmerge-traps -O2 | FileCheck %s --check-prefix=ALL --check-prefix=MERGE
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -emit-llvm -o - -fsanitize=unsigned-integer-overflow -ftrapv -fno-merge-traps -O2 | FileCheck %s --check-prefix=ALL --check-prefix=NOMERGETRAPS
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -emit-llvm -o - -fsanitize=unsigned-integer-overflow -ftrapv -O2| FileCheck %s -check-prefix=ALL --check-prefix=MERGE
+// Verify that -fmerge-traps works as expected with appropriate supporting flags
+
+
+void test_trap_merge() {
+ extern volatile int a, b, c;
+ a = b + c;
+ // MERGE: tail call void @llvm.trap()
+ // NOMERGETRAPS: call void @llvm.trap()
+ b = a + c;
+ // NOMERGETRAPS-DAG: call void @llvm.trap()
+}
+
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -518,6 +518,8 @@
Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
+ Opts.MergeTraps = Args.hasFlag(OPT_fmerge_traps, OPT_fno_merge_traps, true);
+
Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
const Arg *A = Args.getLastArg(OPT_flto, OPT_flto_EQ);
Opts.EmitFunctionSummary = A && A->containsValue("thin");
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -27,6 +27,7 @@
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/InlineAsm.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/MDBuilder.h"
@@ -2535,16 +2536,40 @@
void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
llvm::BasicBlock *Cont = createBasicBlock("cont");
- // If we're optimizing, collapse all calls to trap down to just one per
- // function to save on code size.
- if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ // Collapsing all calls to trap down to one per function makes debugging
+ // these issues much more difficult. Allowing for elimination of this optimization
+ // for debugging purposes.
+ // RE: Bug: 25682
+ if(!CGM.getCodeGenOpts().MergeTraps || !CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+ // If we aren't merging traps, set the function to not be optimized as some
+ // trap calls appear to be merged to the end of the function even with
+ // the call - EmptyAsm - Unreachable pattern at the end of the function.
+ if(!CGM.getCodeGenOpts().MergeTraps) {
+ CurFn->addFnAttr(llvm::Attribute::OptimizeNone);
+ CurFn->addFnAttr(llvm::Attribute::NoInline);
+ }
+
TrapBB = createBasicBlock("trap");
Builder.CreateCondBr(Checked, Cont, TrapBB);
EmitBlock(TrapBB);
- llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
- TrapCall->setDoesNotReturn();
- TrapCall->setDoesNotThrow();
- Builder.CreateUnreachable();
+ if(!CGM.getCodeGenOpts().MergeTraps) {
+ // This closely mirrors what is done to avoid function merging
+ // in the address sanitizer. The trap function is not set
+ // to not return as there is an unreachable instruction
+ // generated at the end of the block.
+ EmitTrapCall(llvm::Intrinsic::trap);
+ llvm::InlineAsm *EmptyAsm = llvm::InlineAsm::get(llvm::FunctionType::get(Builder.getVoidTy(), false),
+ StringRef(""), StringRef(""), true);
+
+ // This stops the trap calls from being merged at the end of the function
+ Builder.CreateCall(EmptyAsm, {});
+ } else {
+ llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+ TrapCall->setDoesNotReturn();
+ TrapCall->setDoesNotThrow();
+ }
+
+ Builder.CreateUnreachable();
} else {
Builder.CreateCondBr(Checked, Cont, TrapBB);
}
Index: include/clang/Frontend/CodeGenOptions.h
===================================================================
--- include/clang/Frontend/CodeGenOptions.h
+++ include/clang/Frontend/CodeGenOptions.h
@@ -151,6 +151,11 @@
/// function instead of to trap instructions.
std::string TrapFuncName;
+ /// Flag controlling whether or not trap calls are merged
+ /// at the end of each function.
+ ///
+ bool MergeTraps;
+
/// A list of command-line options to forward to the LLVM backend.
std::vector<std::string> BackendOptions;
Index: include/clang/Frontend/CodeGenOptions.def
===================================================================
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -77,6 +77,7 @@
///< compile step.
CODEGENOPT(MergeAllConstants , 1, 1) ///< Merge identical constants.
CODEGENOPT(MergeFunctions , 1, 0) ///< Set when -fmerge-functions is enabled.
+CODEGENOPT(MergeTraps , 1, 1) ///< Set when -fmerge-traps is enabled.
CODEGENOPT(MSVolatile , 1, 0) ///< Set when /volatile:ms is enabled.
CODEGENOPT(NoCommon , 1, 0) ///< Set when -fno-common or C++ is enabled.
CODEGENOPT(NoDwarfDirectoryAsm , 1, 0) ///< Set when -fno-dwarf-directory-asm is
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1059,6 +1059,10 @@
def ftrapv_handler : Separate<["-"], "ftrapv-handler">, Group<f_Group>, Flags<[CC1Option]>;
def ftrap_function_EQ : Joined<["-"], "ftrap-function=">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Issue call to specified function rather than a trap instruction">;
+def fmerge_traps : Flag<["-"], "fmerge-traps">, Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Merge all traps to one per function.">;
+def fno_merge_traps : Flag<["-"], "fno-merge-traps">, Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Generate traps inline to aid in debugging.">;
def funit_at_a_time : Flag<["-"], "funit-at-a-time">, Group<f_Group>;
def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
HelpText<"Turn on loop unroller">, Flags<[CC1Option]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
