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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to