Skip to content

Commit 6369ff1

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 (cherry picked from commit 8c4e017) Conflicts: clang/test/BoundsSafety-legacy-checks/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c clang/test/BoundsSafety/CodeGen/unique-trap-blocks-O0-O2-opt-disabled.c
1 parent c8d44a5 commit 6369ff1

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
@@ -332,10 +332,6 @@ CODEGENOPT(ProfileSampleAccurate, 1, 0, Benign) ///< Sample profile is accurate.
332332
/* TO_UPSTREAM(BoundsSafety) ON*/
333333
CODEGENOPT(TrapFuncReturns , 1, 0, Benign) ///< When true, the function specified with
334334
///< -ftrap-function may return normally
335-
CODEGENOPT(
336-
UniqueTrapBlocks, 1,
337-
0, Benign) ///< When true, basic blocks that contain traps they will be prevented
338-
///< from being merged by optimization passes and the backends.
339335
/* TO_UPSTREAM(BoundsSafety) OFF*/
340336

341337
/// 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
@@ -4396,16 +4396,6 @@ defm trap_function_returns : BoolFOption<"trap-function-returns",
43964396
"The trap function specified by -ftrap-function may return normally">,
43974397
NegFlag<SetFalse, [], [ClangOption, CC1Option],
43984398
"The trap function specified by -ftrap-function never returns">>;
4399-
defm unique_traps : BoolFOption<"unique-traps",
4400-
CodeGenOpts<"UniqueTrapBlocks">, DefaultFalse,
4401-
PosFlag<SetTrue, [], [CC1Option],
4402-
"Make trap instructions unique by preventing traps from being merged by the"
4403-
" optimizer. This is useful for preserving the debug information on traps "
4404-
"in optimized code. This makes it easier to debug programs at the cost of "
4405-
"increased code size">,
4406-
NegFlag<SetFalse, [], [CC1Option],
4407-
"Don't try to make trap instructions unique. This allows trap instructions "
4408-
"to be merged by the optimizer.">>;
44094399
/* TO_UPSTREAM(BoundsSafety) OFF */
44104400
def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
44114401
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
@@ -4625,7 +4625,6 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
46254625

46264626
/*TO_UPSTREAM(BoundsSafety) ON*/
46274627
NoMerge |= CGM.getCodeGenOpts().TrapFuncReturns;
4628-
NoMerge |= CGM.getCodeGenOpts().UniqueTrapBlocks;
46294628
/*TO_UPSTREAM(BoundsSafety) OFF*/
46304629

46314630
if (TrapBB && !NoMerge) {
@@ -4654,11 +4653,7 @@ void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
46544653
/* TO_UPSTREAM(BoundsSafety) OFF*/
46554654
} else {
46564655
/*TO_UPSTREAM(BoundsSafety) ON*/
4657-
if (CGM.getCodeGenOpts().UniqueTrapBlocks &&
4658-
!CGM.getCodeGenOpts().TrapFuncReturns)
4659-
TrapBB = createUnmergeableBasicBlock("trap");
4660-
else
4661-
TrapBB = createBasicBlock("trap");
4656+
TrapBB = createBasicBlock("trap");
46624657
auto *BrInst = Builder.CreateCondBr(Checked, Cont, TrapBB,
46634658
MDHelper.createLikelyBranchWeights());
46644659

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"
@@ -321,43 +319,6 @@ TypeEvaluationKind CodeGenFunction::getEvaluationKind(QualType type) {
321319
}
322320
}
323321

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

71047101
// 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)