diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1385,6 +1385,13 @@ let Subjects = SubjectList<[ReturnStmt], ErrorDiag, "return statements">; } +def MustControl : StmtAttr { + let Spellings = [Clang<"mustcontrol">]; + let Documentation = [MustControlDocs]; + let Subjects = SubjectList<[IfStmt, SwitchStmt, ForStmt, WhileStmt, DoStmt], + ErrorDiag, "conditional control statements">; +} + def FastCall : DeclOrTypeAttr { let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">, Keyword<"_fastcall">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -469,6 +469,25 @@ }]; } +def MustControlDocs : Documentation { + let Category = DocCatStmt; + let Content = [{ +Marking a conditional control statement as ``mustcontrol`` indicates that the +compiler must generate a conditional branch in machine code, and such +conditional branch is placed *before* conditionally executed instructions. The +attribute may be ignored if the condition of the control statement is a +constant expression. + +This typically affects optimizations that would cause removal of a conditional +branch. However, it also ensures that a conditional branch and subsequent +instructions are not replaced with non-branching conditional instructions. + +Requesting the generation of a branch may be required in execution environments +where execution of a specific conditional branch inhibits speculation or has +observable side-effects of interest otherwise. + }]; +} + def AssertCapabilityDocs : Documentation { let Category = DocCatFunction; let Heading = "assert_capability, assert_shared_capability"; diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -657,6 +657,7 @@ void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) { bool nomerge = false; const CallExpr *musttail = nullptr; + const Stmt *mustcontrol = nullptr; for (const auto *A : S.getAttrs()) { if (A->getKind() == attr::NoMerge) { @@ -667,9 +668,12 @@ const ReturnStmt *R = cast(Sub); musttail = cast(R->getRetValue()->IgnoreParens()); } + if (A->getKind() == attr::MustControl) + mustcontrol = S.getSubStmt(); } SaveAndRestore save_nomerge(InNoMergeAttributedStmt, nomerge); SaveAndRestore save_musttail(MustTailCall, musttail); + SaveAndRestore save_mustcontrol(MustControlStmt, mustcontrol); EmitStmt(S.getSubStmt(), S.getAttrs()); } @@ -755,10 +759,21 @@ uint64_t Count = getProfileCount(S.getThen()); if (!Count && CGM.getCodeGenOpts().OptimizationLevel) LH = Stmt::getLikelihood(S.getThen(), S.getElse()); - EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH); + EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH, + &S == MustControlStmt); // Emit the 'then' code. EmitBlock(ThenBlock); + if (&S == MustControlStmt) { + // XXX: Implementation subject to change. + // TODO: Make arg unique, so that optimizer doesn't get the bright idea to + // collapse nested mustcontrol statement blocks. + // TODO: Make LLVM emit this during optimization, so that mustcontrol + // doesn't just work for Clang. + Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::sideeffect), + {llvm::ConstantInt::get(IntPtrTy, 42)}); + } + incrementProfileCounter(&S); { RunCleanupsScope ThenScope(*this); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -524,6 +524,10 @@ // applies to. nullptr if there is no 'musttail' on the current statement. const CallExpr *MustTailCall = nullptr; + // The Stmt within the current statement that the mustcontrol attribute + // applies to. nullptr if there is no 'mustcontrol' on the current statement. + const Stmt *MustControlStmt = nullptr; + /// Returns true if a function must make progress, which means the /// mustprogress attribute can be added. bool checkIfFunctionMustProgress() { @@ -4488,7 +4492,8 @@ /// evaluate to true based on PGO data. void EmitBranchOnBoolExpr(const Expr *Cond, llvm::BasicBlock *TrueBlock, llvm::BasicBlock *FalseBlock, uint64_t TrueCount, - Stmt::Likelihood LH = Stmt::LH_None); + Stmt::Likelihood LH = Stmt::LH_None, + bool MustControl = false); /// Given an assignment `*LHS = RHS`, emit a test that checks if \p RHS is /// nonnull, if \p LHS is marked _Nonnull. diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1606,11 +1606,9 @@ /// statement) to the specified blocks. Based on the condition, this might try /// to simplify the codegen of the conditional based on the branch. /// \param LH The value of the likelihood attribute on the True branch. -void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond, - llvm::BasicBlock *TrueBlock, - llvm::BasicBlock *FalseBlock, - uint64_t TrueCount, - Stmt::Likelihood LH) { +void CodeGenFunction::EmitBranchOnBoolExpr( + const Expr *Cond, llvm::BasicBlock *TrueBlock, llvm::BasicBlock *FalseBlock, + uint64_t TrueCount, Stmt::Likelihood LH, bool MustControl) { Cond = Cond->IgnoreParens(); if (const BinaryOperator *CondBOp = dyn_cast(Cond)) { @@ -1795,8 +1793,10 @@ CondV = EvaluateExprAsBool(Cond); } + llvm::MDBuilder MDHelper(getLLVMContext()); llvm::MDNode *Weights = nullptr; llvm::MDNode *Unpredictable = nullptr; + llvm::MDNode *MustControlMD = nullptr; // If the branch has a condition wrapped by __builtin_unpredictable, // create metadata that specifies that the branch is unpredictable. @@ -1804,10 +1804,8 @@ auto *Call = dyn_cast(Cond->IgnoreImpCasts()); if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) { auto *FD = dyn_cast_or_null(Call->getCalleeDecl()); - if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) { - llvm::MDBuilder MDHelper(getLLVMContext()); + if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) Unpredictable = MDHelper.createUnpredictable(); - } } // If there is a Likelihood knowledge for the cond, lower it. @@ -1821,7 +1819,11 @@ Weights = createProfileWeights(TrueCount, CurrentCount - TrueCount); } - Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable); + if (MustControl) + MustControlMD = MDHelper.createMustControl(); + + Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable, + MustControlMD); } /// ErrorUnsupported - Print out an error that codegen doesn't support the diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -233,6 +233,11 @@ return ::new (S.Context) UnlikelyAttr(S.Context, A); } +static Attr *handleMustControl(Sema &S, Stmt *St, const ParsedAttr &A, + SourceRange Range) { + return ::new (S.Context) MustControlAttr(S.Context, A); +} + #define WANT_STMT_MERGE_LOGIC #include "clang/Sema/AttrParsedAttrImpl.inc" #undef WANT_STMT_MERGE_LOGIC @@ -424,6 +429,8 @@ return handleLikely(S, St, A, Range); case ParsedAttr::AT_Unlikely: return handleUnlikely(S, St, A, Range); + case ParsedAttr::AT_MustControl: + return handleMustControl(S, St, A, Range); default: // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a // declaration attribute is not written on a statement, but this code is diff --git a/clang/test/CodeGenCXX/attr-mustcontrol.cpp b/clang/test/CodeGenCXX/attr-mustcontrol.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGenCXX/attr-mustcontrol.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_cc1 -O2 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | FileCheck %s +// RUN: %clang_cc1 -O2 -S -emit-llvm %s -triple x86_64-unknown-linux-gnu -o - | opt -verify +// +// TODO: move to .ll test +// RUN: %clang_cc1 -O2 -S %s -triple aarch64-unknown-linux-gnu -o - | FileCheck %s -check-prefix=ARM + +#if __has_attribute(mustcontrol) +#define mustcontrol [[clang::mustcontrol]] +#else +#error "mustcontrol not supported!" +#endif + +volatile int x, y; +int z; + +// CHECK-LABEL: define{{.*}}IfThenIdenticalWrites +void IfThenIdenticalWrites() { + z = 42; + + // CHECK: br{{.*}}mustcontrol + // ARM: cbz + mustcontrol if (x) { + y = z; + } else { + y = z; + } +} + +// CHECK-LABEL: define{{.*}}SupportWhile +void SupportWhile() { + mustcontrol while (x) { y = 1; } +} + +// CHECK-LABEL: define{{.*}}SupportFor +void SupportFor() { + mustcontrol for (; x; y++) { } +} + +// FIXME: This test currently fails because the optimizer turns the switch into +// a cinc!!! +// +// CHECK-LABEL: define{{.*}}SupportSwitch +// ARM-LABEL: _Z13SupportSwitchv: +void SupportSwitch() { + // ARM-NOT: cinc + mustcontrol switch (x) { + case 0: + y = 1; + break; + default: + y = 2; + break; + } +} diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def --- a/llvm/include/llvm/IR/FixedMetadataKinds.def +++ b/llvm/include/llvm/IR/FixedMetadataKinds.def @@ -42,3 +42,4 @@ LLVM_FIXED_MD_KIND(MD_vcall_visibility, "vcall_visibility", 28) LLVM_FIXED_MD_KIND(MD_noundef, "noundef", 29) LLVM_FIXED_MD_KIND(MD_annotation, "annotation", 30) +LLVM_FIXED_MD_KIND(MD_mustcontrol, "mustcontrol", 31) diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h --- a/llvm/include/llvm/IR/IRBuilder.h +++ b/llvm/include/llvm/IR/IRBuilder.h @@ -934,15 +934,18 @@ //===--------------------------------------------------------------------===// private: - /// Helper to add branch weight and unpredictable metadata onto an - /// instruction. + /// Helper to add branch weight, unpredictable, and mustcontrol metadata onto + /// an instruction. /// \returns The annotated instruction. template - InstTy *addBranchMetadata(InstTy *I, MDNode *Weights, MDNode *Unpredictable) { + InstTy *addBranchMetadata(InstTy *I, MDNode *Weights, MDNode *Unpredictable, + MDNode *MustControl) { if (Weights) I->setMetadata(LLVMContext::MD_prof, Weights); if (Unpredictable) I->setMetadata(LLVMContext::MD_unpredictable, Unpredictable); + if (MustControl) + I->setMetadata(LLVMContext::MD_mustcontrol, MustControl); return I; } @@ -980,9 +983,10 @@ /// instruction. BranchInst *CreateCondBr(Value *Cond, BasicBlock *True, BasicBlock *False, MDNode *BranchWeights = nullptr, - MDNode *Unpredictable = nullptr) { + MDNode *Unpredictable = nullptr, + MDNode *MustControl = nullptr) { return Insert(addBranchMetadata(BranchInst::Create(True, False, Cond), - BranchWeights, Unpredictable)); + BranchWeights, Unpredictable, MustControl)); } /// Create a conditional 'br Cond, TrueDest, FalseDest' @@ -991,9 +995,10 @@ Instruction *MDSrc) { BranchInst *Br = BranchInst::Create(True, False, Cond); if (MDSrc) { - unsigned WL[4] = {LLVMContext::MD_prof, LLVMContext::MD_unpredictable, - LLVMContext::MD_make_implicit, LLVMContext::MD_dbg}; - Br->copyMetadata(*MDSrc, makeArrayRef(&WL[0], 4)); + unsigned WL[5] = {LLVMContext::MD_prof, LLVMContext::MD_unpredictable, + LLVMContext::MD_make_implicit, LLVMContext::MD_dbg, + LLVMContext::MD_mustcontrol}; + Br->copyMetadata(*MDSrc, makeArrayRef(&WL[0], 5)); } return Insert(Br); } @@ -1003,9 +1008,10 @@ /// allocation). SwitchInst *CreateSwitch(Value *V, BasicBlock *Dest, unsigned NumCases = 10, MDNode *BranchWeights = nullptr, - MDNode *Unpredictable = nullptr) { + MDNode *Unpredictable = nullptr, + MDNode *MustControl = nullptr) { return Insert(addBranchMetadata(SwitchInst::Create(V, Dest, NumCases), - BranchWeights, Unpredictable)); + BranchWeights, Unpredictable, MustControl)); } /// Create an indirect branch instruction with the specified address diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td --- a/llvm/include/llvm/IR/Intrinsics.td +++ b/llvm/include/llvm/IR/Intrinsics.td @@ -1326,7 +1326,8 @@ // has having opaque side effects. This may be inserted into loops to ensure // that they are not removed even if they turn out to be empty, for languages // which specify that infinite loops must be preserved. -def int_sideeffect : DefaultAttrsIntrinsic<[], [], [IntrInaccessibleMemOnly, IntrWillReturn]>; +def int_sideeffect : DefaultAttrsIntrinsic<[], [llvm_vararg_ty], + [IntrInaccessibleMemOnly, IntrWillReturn]>; // The pseudoprobe intrinsic works as a place holder to the block it probes. // Like the sideeffect intrinsic defined above, this intrinsic is treated by the diff --git a/llvm/include/llvm/IR/MDBuilder.h b/llvm/include/llvm/IR/MDBuilder.h --- a/llvm/include/llvm/IR/MDBuilder.h +++ b/llvm/include/llvm/IR/MDBuilder.h @@ -66,6 +66,9 @@ /// Return metadata specifying that a branch or switch is unpredictable. MDNode *createUnpredictable(); + /// Return metadata specifying that a branch or switch must not be removed. + MDNode *createMustControl(); + /// Return metadata containing the entry \p Count for a function, a boolean /// \Synthetic indicating whether the counts were synthetized, and the /// GUIDs stored in \p Imports that need to be imported for sample PGO, to diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp --- a/llvm/lib/CodeGen/BranchFolding.cpp +++ b/llvm/lib/CodeGen/BranchFolding.cpp @@ -1217,7 +1217,20 @@ // Blocks should be considered empty if they contain only debug info; // else the debug info would affect codegen. static bool IsEmptyBlock(MachineBasicBlock *MBB) { - return MBB->getFirstNonDebugInstr(true) == MBB->end(); + if (MBB->getFirstNonDebugInstr(true) != MBB->end()) + return false; + + // Even though this block is empty, check if we should preserve it. + // XXX: Implementation subject to change. + if (const auto *BB = MBB->getBasicBlock()) { + for (const BasicBlock *PredBB : predecessors(BB)) { + const auto *PredBr = dyn_cast(PredBB->getTerminator()); + if (PredBr && PredBr->getMetadata(LLVMContext::MD_mustcontrol)) + return false; + } + } + + return true; } // Blocks with only debug info and branches should be considered the same diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp --- a/llvm/lib/IR/IRBuilder.cpp +++ b/llvm/lib/IR/IRBuilder.cpp @@ -960,7 +960,7 @@ if (MDFrom) { MDNode *Prof = MDFrom->getMetadata(LLVMContext::MD_prof); MDNode *Unpred = MDFrom->getMetadata(LLVMContext::MD_unpredictable); - Sel = addBranchMetadata(Sel, Prof, Unpred); + Sel = addBranchMetadata(Sel, Prof, Unpred, nullptr /*TODO*/); } if (isa(Sel)) setFPAttrs(Sel, nullptr /* MDNode* */, FMF); diff --git a/llvm/lib/IR/MDBuilder.cpp b/llvm/lib/IR/MDBuilder.cpp --- a/llvm/lib/IR/MDBuilder.cpp +++ b/llvm/lib/IR/MDBuilder.cpp @@ -56,6 +56,8 @@ return MDNode::get(Context, None); } +MDNode *MDBuilder::createMustControl() { return MDNode::get(Context, None); } + MDNode *MDBuilder::createFunctionEntryCount( uint64_t Count, bool Synthetic, const DenseSet *Imports) {