Tyker updated this revision to Diff 191408.
Tyker added a comment.
Herald added a subscriber: jdoerfert.
added diagnostics for contradictory attributes like for if:
if (...) [[likely]]
return 1;
else [[likely]]
return 2;
handled the codegen for If to generate builtin_expect but i probably didn't do
it the canonical way. i am awaiting advice of how to do it right.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59467/new/
https://reviews.llvm.org/D59467
Files:
clang/include/clang/AST/Stmt.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/ScopeInfo.h
clang/include/clang/Sema/Sema.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+ if (i == 1) [[unlikely]]
+ {
+ return 0;
+ }
+ else if (i == 2) [[likely]]
+ return 1;
+ return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+ One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+ [[likely]] // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+ if (1) [[likely, likely]] {
+ // expected-error@-1 {{there can only be one likely attribue in any attribute list}}
+ // expected-note@-2 {{previously used likely attribue}}
+ [[unlikely]] return ; // expected-error {{'unlikely' attribute can only appear in if, while, switch and for}}
+ }
+ else [[unlikely]] if (1) {
+ while (1) [[likely]] {
+ switch (i) {
+ [[likely]] case 1:
+ default: [[likely]]
+ return ;
+ }
+ }
+ for (;;) [[likely, unlikely]]
+ // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+ // expected-note@-2 {{previously used likely attribue}}
+ [[likely]] return ;
+ // expected-error@-1 {{likely and unlikely are mutually exclusive}}
+ // expected-note@-5 {{previously used unlikely attribue}}
+ }
+ try { // expected-error {{cannot use 'try' with exceptions disabled}}
+ [[likely]]; // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+ } catch (int) {
+ [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+ [[unlikely]] return ;
+ }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaStmtAttr.cpp
===================================================================
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,50 @@
return ::new (S.Context) auto(Attr);
}
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+ SourceRange Range) {
+ LikelihoodAttr Attribute(A.getRange(), S.Context,
+ A.getAttributeSpellingListIndex());
+
+ Scope* scope = S.getCurScope();
+ Scope* previousScope = nullptr;
+
+ if (scope)
+ previousScope = scope->getParent();
+
+ //check that ths attribute is used in an if, while, for, switch or catch
+ if (!previousScope ||
+ !(previousScope->getFlags() & Scope::ControlScope) ||
+ previousScope->getFlags() & Scope::SEHExceptScope ||
+ previousScope->getFlags() & Scope::SEHTryScope ||
+ previousScope->getFlags() & Scope::FnTryCatchScope)
+ S.Diag(A.getLoc(), diag::err_likelihood_outside_control_scope) << A.getName();
+
+ if (!S.getLangOpts().CPlusPlus2a)
+ S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+ clang::Attr* result = ::new (S.Context) auto(Attribute);
+
+ auto* FnScope = S.getCurFunction();
+
+ /// don't need to emit any diagnostics if we aren't in a function or the stack is empty as this is already covered by the checks above
+ if (FnScope && !FnScope->PathLikelyhoodAttrStack.empty()) {
+ Attr*& TopStackAttr = FnScope->PathLikelyhoodAttrStack.back();
+ if (TopStackAttr) {
+ /// only need to compare first character to differenciate between 'likely' and 'unlikely'
+ if (result->getSpelling()[0] == TopStackAttr->getSpelling()[0])
+ S.Diag(result->getLocation(), diag::err_multiple_likelihood) << result->getSpelling();
+ else
+ S.Diag(result->getLocation(), diag::err_mutuably_exclusive_likelihood) << result->getSpelling() << TopStackAttr->getSpelling();
+ S.Diag(TopStackAttr->getLocation(), diag::note_previous_likelihood) << TopStackAttr->getSpelling();
+ } else {
+ TopStackAttr = result;
+ }
+ }
+
+ return result;
+}
+
static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
SourceRange Range) {
if (A.getNumArgs() < 1) {
@@ -336,6 +380,8 @@
return nullptr;
case ParsedAttr::AT_FallThrough:
return handleFallThroughAttr(S, St, A, Range);
+ case ParsedAttr::AT_Likelihood:
+ return handleLikelihoodAttr(S, St, A, Range);
case ParsedAttr::AT_LoopHint:
return handleLoopHintAttr(S, St, A, Range);
case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -525,7 +525,7 @@
Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
ConditionResult Cond,
Stmt *thenStmt, SourceLocation ElseLoc,
- Stmt *elseStmt) {
+ Stmt *elseStmt, IfStmt::Branch expect) {
if (Cond.isInvalid())
Cond = ConditionResult(
*this, nullptr,
@@ -545,13 +545,13 @@
diag::warn_empty_if_body);
return BuildIfStmt(IfLoc, IsConstexpr, InitStmt, Cond, thenStmt, ElseLoc,
- elseStmt);
+ elseStmt, expect);
}
StmtResult Sema::BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr,
Stmt *InitStmt, ConditionResult Cond,
Stmt *thenStmt, SourceLocation ElseLoc,
- Stmt *elseStmt) {
+ Stmt *elseStmt, IfStmt::Branch expect) {
if (Cond.isInvalid())
return StmtError();
@@ -559,7 +559,7 @@
setFunctionHasBranchProtectedScope();
return IfStmt::Create(Context, IfLoc, IsConstexpr, InitStmt, Cond.get().first,
- Cond.get().second, thenStmt, ElseLoc, elseStmt);
+ Cond.get().second, thenStmt, ElseLoc, elseStmt)->setLikelyhood(expect);
}
namespace {
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1197,6 +1197,19 @@
return DC;
}
+
+ void Sema::MaybePushBranchAttrStack() {
+ if (getCurFunction())
+ getCurFunction()->PathLikelyhoodAttrStack.push_back(nullptr);
+ }
+ Attr* Sema::MaybePopBranchAttrStack() {
+ if (!getCurFunction() || getCurFunction()->PathLikelyhoodAttrStack.empty())
+ return nullptr;
+ Attr* result = getCurFunction()->PathLikelyhoodAttrStack.back();
+ getCurFunction()->PathLikelyhoodAttrStack.pop_back();
+ return result;
+ }
+
/// getCurFunctionDecl - If inside of a function body, this returns a pointer
/// to the function decl for the function being parsed. If we're currently
/// in a 'block', this returns the containing context.
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -20,6 +20,9 @@
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/Scope.h"
#include "clang/Sema/TypoCorrection.h"
+#include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/SemaDiagnostic.h"
+
using namespace clang;
//===----------------------------------------------------------------------===//
@@ -1245,6 +1248,7 @@
SourceLocation InnerStatementTrailingElseLoc;
StmtResult ThenStmt;
+ Actions.MaybePushBranchAttrStack();
{
EnterExpressionEvaluationContext PotentiallyDiscarded(
Actions, Sema::ExpressionEvaluationContext::DiscardedStatement, nullptr,
@@ -1253,6 +1257,8 @@
ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc);
}
+ Attr* ThenBranchAttr = Actions.MaybePopBranchAttrStack();
+
// Pop the 'if' scope if needed.
InnerScope.Exit();
@@ -1279,6 +1285,7 @@
//
ParseScope InnerScope(this, Scope::DeclScope, C99orCXX,
Tok.is(tok::l_brace));
+ Actions.MaybePushBranchAttrStack();
EnterExpressionEvaluationContext PotentiallyDiscarded(
Actions, Sema::ExpressionEvaluationContext::DiscardedStatement, nullptr,
@@ -1296,7 +1303,7 @@
Diag(InnerStatementTrailingElseLoc, diag::warn_dangling_else);
}
- IfScope.Exit();
+ Attr* ElseBranchAttr = Actions.MaybePopBranchAttrStack();
// If the then or else stmt is invalid and the other is valid (and present),
// make turn the invalid one into a null stmt to avoid dropping the other
@@ -1308,6 +1315,23 @@
return StmtError();
}
+ IfStmt::Branch WhichBranch = IfStmt::None;
+ if (ThenBranchAttr) {
+ if (ElseBranchAttr && ThenBranchAttr->getSpelling()[0] == ElseBranchAttr->getSpelling()[0]) {
+ Diag(ElseBranchAttr->getLocation(), diag::err_contradictory_else_likelihood) << ElseBranchAttr->getSpelling();
+ Diag(ThenBranchAttr->getLocation(), diag::note_previous_likelihood) << ThenBranchAttr->getSpelling();
+ }
+ if (ThenBranchAttr->getSpelling()[0] == 'l')
+ WhichBranch = IfStmt::ThenBranch;
+ else
+ WhichBranch = IfStmt::ElseBranch;
+ } else if (ElseBranchAttr) {
+ if (ThenBranchAttr->getSpelling()[0] == 'l')
+ WhichBranch = IfStmt::ElseBranch;
+ else
+ WhichBranch = IfStmt::ThenBranch;
+ }
+
// Now if either are invalid, replace with a ';'.
if (ThenStmt.isInvalid())
ThenStmt = Actions.ActOnNullStmt(ThenStmtLoc);
@@ -1315,7 +1339,7 @@
ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+ ThenStmt.get(), ElseLoc, ElseStmt.get(), WhichBranch);
}
/// ParseSwitchStatement
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4041,7 +4041,7 @@
/// TrueCount should be the number of times we expect the condition to
/// evaluate to true based on PGO data.
void EmitBranchOnBoolExpr(const Expr *Cond, llvm::BasicBlock *TrueBlock,
- llvm::BasicBlock *FalseBlock, uint64_t TrueCount);
+ llvm::BasicBlock *FalseBlock, uint64_t TrueCount, IfStmt::Branch expect = IfStmt::None);
/// 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
@@ -1531,7 +1531,8 @@
void CodeGenFunction::EmitBranchOnBoolExpr(const Expr *Cond,
llvm::BasicBlock *TrueBlock,
llvm::BasicBlock *FalseBlock,
- uint64_t TrueCount) {
+ uint64_t TrueCount,
+ IfStmt::Branch expect) {
Cond = Cond->IgnoreParens();
if (const BinaryOperator *CondBOp = dyn_cast<BinaryOperator>(Cond)) {
@@ -1720,6 +1721,13 @@
ApplyDebugLocation DL(*this, Cond);
CondV = EvaluateExprAsBool(Cond);
}
+
+ if (expect != IfStmt::None && CGM.getCodeGenOpts().OptimizationLevel != 0)
+ {
+ llvm::Constant* ExpectedValue = llvm::ConstantInt::get(llvm::Type::getInt1Ty(this->getLLVMContext()), expect == IfStmt::ThenBranch);
+ llvm::Function *FnExpect = CGM.getIntrinsic(llvm::Intrinsic::expect, CondV->getType());
+ Builder.CreateCall(FnExpect, {CondV, ExpectedValue}, "expval");
+ }
Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable);
}
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -658,7 +658,7 @@
ElseBlock = createBasicBlock("if.else");
EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock,
- getProfileCount(S.getThen()));
+ getProfileCount(S.getThen()), S.getLikelyhood());
// Emit the 'then' code.
EmitBlock(ThenBlock);
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3841,14 +3841,18 @@
Stmt *SubStmt);
class ConditionResult;
+
+ void MaybePushBranchAttrStack();
+ Attr* MaybePopBranchAttrStack();
+
StmtResult ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr,
Stmt *InitStmt,
ConditionResult Cond, Stmt *ThenVal,
- SourceLocation ElseLoc, Stmt *ElseVal);
+ SourceLocation ElseLoc, Stmt *ElseVal, IfStmt::Branch expect = IfStmt::None);
StmtResult BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr,
Stmt *InitStmt,
ConditionResult Cond, Stmt *ThenVal,
- SourceLocation ElseLoc, Stmt *ElseVal);
+ SourceLocation ElseLoc, Stmt *ElseVal, IfStmt::Branch expect = IfStmt::None);
StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
Stmt *InitStmt,
ConditionResult Cond);
Index: clang/include/clang/Sema/ScopeInfo.h
===================================================================
--- clang/include/clang/Sema/ScopeInfo.h
+++ clang/include/clang/Sema/ScopeInfo.h
@@ -183,6 +183,11 @@
/// block.
SmallVector<SwitchInfo, 8> SwitchStack;
+ /// ControlScopeStack - This is the current set of active ControlScrutures in the
+ /// Control structures are if, while, for, switch, catch
+ /// block.
+ SmallVector<Attr*, 8> PathLikelyhoodAttrStack;
+
/// The list of return statements that occur within the function or
/// block, if there is any chance of applying the named return value
/// optimization, or if we need to infer a return type.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
"use of the %0 attribute is a C++14 extension">, InGroup<CXX14>;
def ext_cxx17_attr : Extension<
"use of the %0 attribute is a C++17 extension">, InGroup<CXX17>;
+def ext_cxx2a_attr : Extension<
+ "use of the %0 attribute is a C++2a extension">, InGroup<CXX2a>;
def warn_unused_comparison : Warning<
"%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,17 @@
"fallthrough annotation in unreachable code">,
InGroup<ImplicitFallthrough>, DefaultIgnore;
+def err_likelihood_outside_control_scope : Error<
+ "%0 attribute can only appear inside an if, while, for, switch or catch">;
+def err_multiple_likelihood : Error<
+ "there can only be one %0 attribue in any attribute list">;
+def err_mutuably_exclusive_likelihood : Error<
+ "%0 and %1 are mutually exclusive">;
+def err_contradictory_else_likelihood : Error<
+ "else branch has attribute %0 contradicing with previous attribute">;
+def note_previous_likelihood : Note<
+ "previously used %0 attribue">;
+
def warn_unreachable_default : Warning<
"default label in switch which covers all enumeration values">,
InGroup<CoveredSwitchDefault>, DefaultIgnore;
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1492,6 +1492,39 @@
}];
}
+def LikelihoodDocs : Documentation {
+ let Category = DocCatStmt;
+ let Heading = "likely / unlikely";
+ let Content = [{
+
+The ``likely`` or ``unlikely`` attribute is used to annotate that a statement or label is likely or unlikely to executed
+
+Here is an example:
+
+.. code-block:: c++
+
+ void g(int);
+ int f(int n) {
+ if (n > 5) [[unlikely]] { // n > 5 is considered to be arbitrarily unlikely
+ g(0);
+ return n * 2 + 1;
+ }
+
+ switch (n) {
+ case 1:
+ g(1);
+ [[fallthrough]];
+
+ [[likely]] case 2: // n == 2 is considered to be arbitrarily more
+ g(2); // likely than any other value of n
+ break;
+ }
+ return 3;
+ }
+
+ }];
+}
+
def ARMInterruptDocs : Documentation {
let Category = DocCatFunction;
let Heading = "interrupt (ARM)";
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1147,6 +1147,12 @@
let Documentation = [FallthroughDocs];
}
+def Likelihood : StmtAttr {
+ let Spellings = [CXX11<"", "likely", 201803>, Clang<"likely">, CXX11<"", "unlikely", 201803>, Clang<"unlikely">];
+// let Subjects = [Stmt, LabelStmt];
+ let Documentation = [LikelihoodDocs];
+}
+
def FastCall : DeclOrTypeAttr {
let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">,
Keyword<"_fastcall">];
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -1758,6 +1758,27 @@
explicit IfStmt(EmptyShell Empty, bool HasElse, bool HasVar, bool HasInit);
public:
+/// Indicate wether the likelihood attribute is on the Then or Else branch
+ enum Branch {
+ None = 0,
+ ThenBranch = 1,
+ ElseBranch = 2
+ };
+
+private:
+ Branch Likelihood = None;
+public:
+
+
+ IfStmt* setLikelyhood(Branch branch) {
+ Likelihood = branch;
+ return this;
+ }
+
+ Branch getLikelyhood() const {
+ return Likelihood;
+ }
+
/// Create an IfStmt.
static IfStmt *Create(const ASTContext &Ctx, SourceLocation IL,
bool IsConstexpr, Stmt *Init, VarDecl *Var, Expr *Cond,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
