Skip to content

Commit 8c4e017

Browse files
committed
[BoundsSafety] Remove -funique-traps flag and its implementation
The implementation was a bit hacky and interacted badly with the hot-cold splitting pass. So it is going to be removed in favor of a different implementation that's simpler. I also investated why `clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O2.c` broke (rdar://150559639). It turns outthat the "trap blocks" started being unintentionally split by the hot-cold splitting pass because in upstream we started adding branch profiling weights (llvm#134310) which the hot-cold splitting pass used as a signal to split the "trap blocks" into their own function (by-passing the logic I originally added to prevent the splitting). While we could fix this it would be very fragile and given that we are removing the implementation anyway it doesn't matter anymore. rdar://158088410
1 parent 5279332 commit 8c4e017

File tree

12 files changed

+2
-516
lines changed

12 files changed

+2
-516
lines changed

clang/include/clang/Basic/CodeGenOptions.def

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,6 @@ CODEGENOPT(ProfileSampleAccurate, 1, 0, Benign) ///< Sample profile is accurate.
336336
/* TO_UPSTREAM(BoundsSafety) ON*/
337337
CODEGENOPT(TrapFuncReturns , 1, 0, Benign) ///< When true, the function specified with
338338
///< -ftrap-function may return normally
339-
CODEGENOPT(
340-
UniqueTrapBlocks, 1,
341-
0, Benign) ///< When true, basic blocks that contain traps they will be prevented
342-
///< from being merged by optimization passes and the backends.
343339
/* TO_UPSTREAM(BoundsSafety) OFF*/
344340

345341
/// Treat loops as finite: language, always, never.

clang/include/clang/Driver/Options.td

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4445,16 +4445,6 @@ defm trap_function_returns : BoolFOption<"trap-function-returns",
44454445
"The trap function specified by -ftrap-function may return normally">,
44464446
NegFlag<SetFalse, [], [ClangOption, CC1Option],
44474447
"The trap function specified by -ftrap-function never returns">>;
4448-
defm unique_traps : BoolFOption<"unique-traps",
4449-
CodeGenOpts<"UniqueTrapBlocks">, DefaultFalse,
4450-
PosFlag<SetTrue, [], [CC1Option],
4451-
"Make trap instructions unique by preventing traps from being merged by the"
4452-
" optimizer. This is useful for preserving the debug information on traps "
4453-
"in optimized code. This makes it easier to debug programs at the cost of "
4454-
"increased code size">,
4455-
NegFlag<SetFalse, [], [CC1Option],
4456-
"Don't try to make trap instructions unique. This allows trap instructions "
4457-
"to be merged by the optimizer.">>;
44584448
/* TO_UPSTREAM(BoundsSafety) OFF */
44594449
def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
44604450
HelpText<"Turn on loop unroller">, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4712,7 +4712,6 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
47124712

47134713
/*TO_UPSTREAM(BoundsSafety) ON*/
47144714
NoMerge |= CGM.getCodeGenOpts().TrapFuncReturns;
4715-
NoMerge |= CGM.getCodeGenOpts().UniqueTrapBlocks;
47164715
/*TO_UPSTREAM(BoundsSafety) OFF*/
47174716

47184717
if (TrapBB && !NoMerge) {
@@ -4741,11 +4740,7 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
47414740
/* TO_UPSTREAM(BoundsSafety) OFF*/
47424741
} else {
47434742
/*TO_UPSTREAM(BoundsSafety) ON*/
4744-
if (CGM.getCodeGenOpts().UniqueTrapBlocks &&
4745-
!CGM.getCodeGenOpts().TrapFuncReturns)
4746-
TrapBB = createUnmergeableBasicBlock("trap");
4747-
else
4748-
TrapBB = createBasicBlock("trap");
4743+
TrapBB = createBasicBlock("trap");
47494744
auto *BrInst = Builder.CreateCondBr(Checked, Cont, TrapBB,
47504745
MDHelper.createLikelyBranchWeights());
47514746

clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@
4141
#include "llvm/IR/DataLayout.h"
4242
#include "llvm/IR/Dominators.h"
4343
#include "llvm/IR/FPEnv.h"
44-
// TO_UPSTREAM(BoundsSafety) ON
45-
#include "llvm/IR/InlineAsm.h"
4644
#include "llvm/IR/Instruction.h"
4745
#include "llvm/IR/IntrinsicInst.h"
4846
#include "llvm/IR/Intrinsics.h"
@@ -320,43 +318,6 @@ TypeEvaluationKind CodeGenFunction::getEvaluationKind(QualType type) {
320318
}
321319
}
322320

323-
/* TO_UPSTREAM(BoundsSafety) ON*/
324-
llvm::BasicBlock *CodeGenFunction::createUnmergeableBasicBlock(
325-
const Twine &name, llvm::Function *parent, llvm::BasicBlock *before) {
326-
auto *BB = createBasicBlock(name, parent, before);
327-
// This approach is the same approach used by Swift.
328-
// TODO: Find a better way to do this (rdar://137627723).
329-
330-
// Emit unique side-effecting inline asm calls in order to eliminate
331-
// the possibility that an LLVM optimization or code generation pass
332-
// will merge these blocks back together again. We emit an empty asm
333-
// string with the side-effect flag set, and with a unique integer
334-
// argument.
335-
llvm::IntegerType *asmArgTy = CGM.Int64Ty;
336-
llvm::Type *argTys = {asmArgTy};
337-
llvm::FunctionType *asmFnTy =
338-
llvm::FunctionType::get(CGM.VoidTy, argTys, /* isVarArg=*/false);
339-
// "n" is an input constraint stating that the first argument to the call
340-
// will be an integer literal.
341-
llvm::InlineAsm *inlineAsm =
342-
llvm::InlineAsm::get(asmFnTy, /*AsmString=*/"", /*Constraints=*/"n",
343-
/*hasSideEffects=*/true);
344-
345-
// Use the builder so that any opt-remarks and attributes are automatically
346-
// applied by the builder. The current state of the builder is saved so it
347-
// can be used for creating the asm call and then the builder is reset to
348-
// its previous state.
349-
auto OldInsertPoint = Builder.GetInsertPoint();
350-
auto* OldInsertBB = Builder.GetInsertBlock();
351-
Builder.SetInsertPoint(BB);
352-
Builder.CreateCall(
353-
inlineAsm,
354-
llvm::ConstantInt::get(asmArgTy, CGM.getAndIncrementUniqueTrapCount()));
355-
Builder.SetInsertPoint(OldInsertBB, OldInsertPoint);
356-
return BB;
357-
}
358-
/* TO_UPSTREAM(BoundsSafety) OFF*/
359-
360321
llvm::DebugLoc CodeGenFunction::EmitReturnBlock() {
361322
// For cleanliness, we try to avoid emitting the return block for
362323
// simple cases.

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,13 +2659,6 @@ class CodeGenFunction : public CodeGenTypeCache {
26592659
return llvm::BasicBlock::Create(getLLVMContext(), name, parent, before);
26602660
}
26612661

2662-
/* TO_UPSTREM(BoundsSafety) ON*/
2663-
llvm::BasicBlock *
2664-
createUnmergeableBasicBlock(const Twine &name = "",
2665-
llvm::Function *parent = nullptr,
2666-
llvm::BasicBlock *before = nullptr);
2667-
/* TO_UPSTREM(BoundsSafety) OFF*/
2668-
26692662
/// getBasicBlockForLabel - Return the LLVM basicblock that the specified
26702663
/// label maps to.
26712664
JumpDest getJumpDestForLabel(const LabelDecl *S);

clang/lib/CodeGen/CodeGenModule.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -682,9 +682,6 @@ class CodeGenModule : public CodeGenTypeCache {
682682
std::optional<PointerAuthQualifier>
683683
computeVTPointerAuthentication(const CXXRecordDecl *ThisClass);
684684

685-
// TO_UPSTREAM(BoundsSafety)
686-
uint64_t UniqueTrapCount = 0;
687-
688685
AtomicOptions AtomicOpts;
689686

690687
// A set of functions which should be hot-patched; see
@@ -1159,11 +1156,6 @@ class CodeGenModule : public CodeGenTypeCache {
11591156
/// Fetches the global unique block count.
11601157
int getUniqueBlockCount() { return ++Block.GlobalUniqueCount; }
11611158

1162-
/* TO_UPSTREAM(BoundsSafety) ON*/
1163-
/// Fetches and increments unique trap count
1164-
uint64_t getAndIncrementUniqueTrapCount() { return UniqueTrapCount++; }
1165-
/* TO_UPSTREAM(BoundsSafety) OFF*/
1166-
11671159
/// Fetches the type of a generic block descriptor.
11681160
llvm::Type *getBlockDescriptorType();
11691161

clang/lib/Driver/ToolChains/Clang.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7041,9 +7041,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &Job,
70417041
D.Diag(diag::err_drv_option_requires_option)
70427042
<< "-ftrap-function-returns" << "-ftrap-function=";
70437043
}
7044-
7045-
Args.addOptInFlag(CmdArgs, options::OPT_funique_traps,
7046-
options::OPT_fno_unique_traps);
70477044
/* TO_UPSTREAM(BoundsSafety) OFF*/
70487045

70497046
// Handle -f[no-]wrapv and -f[no-]strict-overflow, which are used by both

clang/test/BoundsSafety-legacy-checks/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c

Lines changed: 0 additions & 105 deletions
This file was deleted.

0 commit comments

Comments
 (0)