melver created this revision.
Herald added subscribers: dexonsmith, jfb, hiraditya.
Herald added a reviewer: aaron.ballman.
melver requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert.
Herald added projects: clang, LLVM.
[ WIP, only high-level comments for now ]
Introduce a new attribute, 'MustControl'/'mustcontrol', which denotes that a
conditional control statement must result in true control-flow and not
be optimized away. The attribute otherwise has no semantic relevance.
However, the existence of a true branch is of relevance when branch
execution has side-effects on machine state that the programmer is
interested in, for example in OS kernels.
The Linux kernel, for one, relies on the existence of true conditional
branches for the enforcement of memory orders, per Linux-kernel memory
consistency model (LKMM) [1]. With the 'mustcontrol' attribute, Clang
would provide a primitive required for the Linux kernel to ensure a true
branch is emitted without resorting to inline assembly (which often
results in poor codegen). The primitive is simple and low-level enough,
that the compiler can remain blissfully unaware of the LKMM and leave
the semantics of Linux's memory model to the kernel community.
[1] https://lkml.kernel.org/r/yln8dzbnwvqrq...@hirez.programming.kicks-ass.net
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103958
Files:
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/CodeGenCXX/attr-mustcontrol.cpp
llvm/include/llvm/IR/FixedMetadataKinds.def
llvm/include/llvm/IR/IRBuilder.h
llvm/include/llvm/IR/Intrinsics.td
llvm/include/llvm/IR/MDBuilder.h
llvm/lib/CodeGen/BranchFolding.cpp
llvm/lib/IR/IRBuilder.cpp
llvm/lib/IR/MDBuilder.cpp
Index: llvm/lib/IR/MDBuilder.cpp
===================================================================
--- llvm/lib/IR/MDBuilder.cpp
+++ 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<GlobalValue::GUID> *Imports) {
Index: llvm/lib/IR/IRBuilder.cpp
===================================================================
--- llvm/lib/IR/IRBuilder.cpp
+++ 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<FPMathOperator>(Sel))
setFPAttrs(Sel, nullptr /* MDNode* */, FMF);
Index: llvm/lib/CodeGen/BranchFolding.cpp
===================================================================
--- llvm/lib/CodeGen/BranchFolding.cpp
+++ llvm/lib/CodeGen/BranchFolding.cpp
@@ -1217,7 +1217,19 @@
// 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.
+ if (const auto *BB = MBB->getBasicBlock()) {
+ for (const BasicBlock *PredBB : predecessors(BB)) {
+ const auto *PredBr = dyn_cast<BranchInst>(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
Index: llvm/include/llvm/IR/MDBuilder.h
===================================================================
--- llvm/include/llvm/IR/MDBuilder.h
+++ 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
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ 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
Index: llvm/include/llvm/IR/IRBuilder.h
===================================================================
--- llvm/include/llvm/IR/IRBuilder.h
+++ 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 <typename InstTy>
- 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
Index: llvm/include/llvm/IR/FixedMetadataKinds.def
===================================================================
--- llvm/include/llvm/IR/FixedMetadataKinds.def
+++ 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)
Index: clang/test/CodeGenCXX/attr-mustcontrol.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/attr-mustcontrol.cpp
@@ -0,0 +1,27 @@
+// 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
+// FIXME: remove the call to "opt" once the tests are running the Clang verifier automatically again.
+
+#if __has_attribute(mustcontrol)
+void HasAttribute() { }
+#endif
+
+volatile int x, y;
+int z;
+
+void IdenticalWrites(int x) {
+ z = 42;
+
+ [[clang::mustcontrol]] if (x) {
+ y = z;
+ } else {
+ y = z;
+ }
+}
+// TODO: CHECK...
+
+
+// Test spellings
+void GNUSpelling() {
+ __attribute__((mustcontrol)) if (x) { }
+}
Index: clang/lib/Sema/SemaStmtAttr.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ 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
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ 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() {
@@ -4479,7 +4483,7 @@
llvm::BasicBlock *FalseBlock,
uint64_t TrueCount = 0,
Stmt::Likelihood LH = Stmt::LH_None,
- const Expr *CntrIdx = nullptr);
+ const Expr *CntrIdx = nullptr); // TODO
/// EmitBranchOnBoolExpr - Emit a branch on a boolean condition (e.g. for an
/// if statement) to the specified blocks. Based on the condition, this might
@@ -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.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ 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<BinaryOperator>(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<CallExpr>(Cond->IgnoreImpCasts());
if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) {
auto *FD = dyn_cast_or_null<FunctionDecl>(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
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ 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,13 @@
const ReturnStmt *R = cast<ReturnStmt>(Sub);
musttail = cast<CallExpr>(R->getRetValue()->IgnoreParens());
}
+ if (A->getKind() == attr::MustControl)
+ mustcontrol = S.getSubStmt();
}
+
SaveAndRestore<bool> save_nomerge(InNoMergeAttributedStmt, nomerge);
SaveAndRestore<const CallExpr *> save_musttail(MustTailCall, musttail);
+ SaveAndRestore<const Stmt *> save_mustcontrol(MustControlStmt, mustcontrol);
EmitStmt(S.getSubStmt(), S.getAttrs());
}
@@ -755,10 +760,20 @@
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) {
+ // 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);
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ 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 = [MustTailDocs]; // TODO
+ let Subjects = SubjectList<[IfStmt, SwitchStmt, ForStmt, WhileStmt, DoStmt],
+ ErrorDiag, "conditional control statements">;
+}
+
def FastCall : DeclOrTypeAttr {
let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">,
Keyword<"_fastcall">];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
