Skip to content

[Clang][C++23] Implement P1774R8: Portable assumptions #81014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Mar 9, 2024

Conversation

Sirraide
Copy link
Member

@Sirraide Sirraide commented Feb 7, 2024

This implements the C++23 assume attribute.

This pr currently does not include any changes to constant evaluation. Even though the standard specifies that the behaviour is undefined should an assumption evaluate to false at runtime [dcl.attr.assume], it also mentions that we’re not required to diagnose that [expr.const], as I understand it.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-clang-codegen

Author: None (Sirraide)

Changes

This implements the C++23 assume attribute.

This pr currently does not include any changes to constant evaluation. Even though the standard specifies that the behaviour is undefined should an assumption evaluate to false at runtime [dcl.attr.assume], it also mentions that we’re not required to diagnose that [expr.const], as I understand it.


Full diff: https://github.com/llvm/llvm-project/pull/81014.diff

14 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/include/clang/Basic/Attr.td (+9)
  • (modified) clang/include/clang/Basic/AttrDocs.td (+24)
  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-1)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7)
  • (modified) clang/include/clang/Parse/Parser.h (+7)
  • (modified) clang/lib/CodeGen/CGStmt.cpp (+9-2)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+54-1)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+13)
  • (modified) clang/lib/Sema/SemaStmtAttr.cpp (+37)
  • (added) clang/test/CodeGenCXX/cxx23-assume.cpp (+28)
  • (added) clang/test/Parser/cxx23-assume.cpp (+14)
  • (added) clang/test/SemaCXX/cxx23-assume.cpp (+50)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 802c44b6c86080..6312e100913613 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -95,6 +95,7 @@ C++23 Feature Support
 
 - Implemented `P2718R0: Lifetime extension in range-based for loops <https://wg21.link/P2718R0>`_. Also
   materialize temporary object which is a prvalue in discarded-value expression.
+- Implemented `P1774R8: Portable assumptions <https://wg21.link/P1774R8>`_.
 
 C++2c Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b2d5309e142c1a..6cd2a541da17d9 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1564,6 +1564,15 @@ def Unlikely : StmtAttr {
 }
 def : MutualExclusions<[Likely, Unlikely]>;
 
+def Assume : StmtAttr {
+  let Spellings = [CXX11<"", "assume", 202302>];
+  let Subjects = SubjectList<[NullStmt], ErrorDiag, "empty statements">;
+  // The standard only allows a conditional-expression here, but we ought
+  // to get better results by handling that in Sema.
+  let Args = [ExprArgument<"Assumption">];
+  let Documentation = [AssumeDocs];
+}
+
 def NoMerge : DeclOrStmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 041786f37fb8a7..1b27a46ef1d901 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1996,6 +1996,30 @@ Here is an example:
   }];
 }
 
+def AssumeDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "assume";
+  let Content = [{
+The ``assume`` attribute is used to indicate to the optimizer that a
+certain condition can be assumed to be true at a certain point in the
+program. If this condition is violated at runtime, the behavior is
+undefined. ``assume`` can only be applied to a null statement.
+
+Note that `clang::assume` is a different attribute. Always write ``assume``
+without a namespace if you intend to use the standard C++ attribute.
+
+Example:
+
+.. code-block:: c++
+
+  int f(int x, int y) {
+    [[assume(x == 27)]];
+    [[assume(x == y)]];
+    return y + 1; // Will be optimised to `return 28`.
+  }
+  }];
+}
+
 def LikelihoodDocs : Documentation {
   let Category = DocCatStmt;
   let Heading = "likely and unlikely";
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6765721ae7002c..192b081404a827 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1124,9 +1124,11 @@ def NonGCC : DiagGroup<"non-gcc",
 def CXX14Attrs : DiagGroup<"c++14-attribute-extensions">;
 def CXX17Attrs : DiagGroup<"c++17-attribute-extensions">;
 def CXX20Attrs : DiagGroup<"c++20-attribute-extensions">;
+def CXX23Attrs : DiagGroup<"c++23-attribute-extensions">;
 def FutureAttrs : DiagGroup<"future-attribute-extensions", [CXX14Attrs,
                                                             CXX17Attrs,
-                                                            CXX20Attrs]>;
+                                                            CXX20Attrs,
+                                                            CXX23Attrs]>;
 
 def CXX23AttrsOnLambda : DiagGroup<"c++23-lambda-attributes">;
 
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index a30ab27566ec3e..9ecfdab3617e05 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -783,6 +783,9 @@ def err_ms_property_expected_comma_or_rparen : Error<
 def err_ms_property_initializer : Error<
   "property declaration cannot have a default member initializer">;
 
+def err_assume_attr_expects_cond_expr : Error<
+  "use of this expression in an 'assume' attribute requires parentheses">;
+
 def warn_cxx20_compat_explicit_bool : Warning<
   "this expression will be parsed as explicit(bool) in C++20">,
   InGroup<CXX20Compat>, DefaultIgnore;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b4dc4feee8e63a..847168af288622 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9083,6 +9083,8 @@ def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup<CXX17Attrs>;
 def ext_cxx20_attr : Extension<
   "use of the %0 attribute is a C++20 extension">, InGroup<CXX20Attrs>;
+def ext_cxx23_attr : Extension<
+  "use of the %0 attribute is a C++23 extension">, InGroup<CXX23Attrs>;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -10149,6 +10151,11 @@ def err_fallthrough_attr_outside_switch : Error<
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
 
+def err_assume_attr_args : Error<
+  "attribute 'assume' requires a single expression argument">;
+def err_assume_attr_wrong_target : Error<
+  "'assume' attribute is only allowed on empty statements">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup<CoveredSwitchDefault>, DefaultIgnore;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..0f982dbb67b41c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1801,6 +1801,7 @@ class Parser : public CodeCompletionHandler {
   ExprResult ParseConstraintLogicalOrExpression(bool IsTrailingRequiresClause);
   // Expr that doesn't include commas.
   ExprResult ParseAssignmentExpression(TypeCastState isTypeCast = NotTypeCast);
+  ExprResult ParseConditionalExpression();
 
   ExprResult ParseMSAsmIdentifier(llvm::SmallVectorImpl<Token> &LineToks,
                                   unsigned &NumLineToksConsumed,
@@ -2953,6 +2954,12 @@ class Parser : public CodeCompletionHandler {
                                SourceLocation ScopeLoc,
                                CachedTokens &OpenMPTokens);
 
+  /// Parse a C++23 assume() attribute. Returns true on error.
+  bool ParseAssumeAttributeArg(ParsedAttributes &Attrs,
+                               IdentifierInfo *AttrName,
+                               SourceLocation AttrNameLoc,
+                               SourceLocation *EndLoc);
+
   IdentifierInfo *TryParseCXX11AttributeIdentifier(
       SourceLocation &Loc,
       Sema::AttributeCompletion Completion = Sema::AttributeCompletion::None,
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index beff0ad9da2709..66be305550b1b9 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -721,11 +721,18 @@ void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
     case attr::AlwaysInline:
       alwaysinline = true;
       break;
-    case attr::MustTail:
+    case attr::MustTail: {
       const Stmt *Sub = S.getSubStmt();
       const ReturnStmt *R = cast<ReturnStmt>(Sub);
       musttail = cast<CallExpr>(R->getRetValue()->IgnoreParens());
-      break;
+    } break;
+    case attr::Assume: {
+      const Expr *Assumption = cast<AssumeAttr>(A)->getAssumption();
+      if (!Assumption->HasSideEffects(getContext())) {
+        llvm::Value *AssumptionVal = EvaluateExprAsBool(Assumption);
+        Builder.CreateAssumption(AssumptionVal);
+      }
+    } break;
     }
   }
   SaveAndRestore save_nomerge(InNoMergeAttributedStmt, nomerge);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 79928ddb5af599..7f2762d4fe22b9 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4528,6 +4528,54 @@ static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
   }
 }
 
+/// Parse the argument to C++23's [[assume()]] attribute.
+bool Parser::ParseAssumeAttributeArg(ParsedAttributes &Attrs,
+                                     IdentifierInfo *AttrName,
+                                     SourceLocation AttrNameLoc,
+                                     SourceLocation *EndLoc) {
+  assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");
+  BalancedDelimiterTracker T(*this, tok::l_paren);
+  T.consumeOpen();
+
+  // [dcl.attr.assume]: The expression is potentially evaluated.
+  EnterExpressionEvaluationContext Unevaluated(
+      Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+
+  TentativeParsingAction TPA(*this);
+  ExprResult Res(
+      Actions.CorrectDelayedTyposInExpr(ParseConditionalExpression()));
+  if (Res.isInvalid() || !Tok.is(tok::r_paren)) {
+    // Emit a better diagnostic if this is an otherwise valid expression that
+    // is not allowed here.
+    TPA.Revert();
+    Sema::TentativeAnalysisScope Scope(Actions);
+    Res = ParseExpression();
+    if (!Res.isInvalid()) {
+      auto *E = Res.get();
+      Diag(E->getExprLoc(), diag::err_assume_attr_expects_cond_expr)
+          << AttrName << FixItHint::CreateInsertion(E->getBeginLoc(), "(")
+          << FixItHint::CreateInsertion(PP.getLocForEndOfToken(E->getEndLoc()),
+                                        ")")
+          << E->getSourceRange();
+    }
+
+    T.consumeClose();
+    return true;
+  }
+
+  TPA.Commit();
+  ArgsUnion Assumption = Res.get();
+  auto RParen = Tok.getLocation();
+  T.consumeClose();
+  Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), nullptr,
+               SourceLocation(), &Assumption, 1, ParsedAttr::Form::CXX11());
+
+  if (EndLoc)
+    *EndLoc = RParen;
+
+  return false;
+}
+
 /// ParseCXX11AttributeArgs -- Parse a C++11 attribute-argument-clause.
 ///
 /// [C++11] attribute-argument-clause:
@@ -4596,7 +4644,12 @@ bool Parser::ParseCXX11AttributeArgs(
   if (ScopeName && (ScopeName->isStr("clang") || ScopeName->isStr("_Clang")))
     NumArgs = ParseClangAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc,
                                       ScopeName, ScopeLoc, Form);
-  else
+  // So does C++23's assume() attribute.
+  else if (!ScopeName && AttrName->isStr("assume")) {
+    if (ParseAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc))
+      return true;
+    NumArgs = 1;
+  } else
     NumArgs = ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,
                                        ScopeName, ScopeLoc, Form);
 
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 52cebdb6f64bac..cf2a7bd026c5de 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -179,6 +179,19 @@ ExprResult Parser::ParseAssignmentExpression(TypeCastState isTypeCast) {
   return ParseRHSOfBinaryExpression(LHS, prec::Assignment);
 }
 
+ExprResult Parser::ParseConditionalExpression() {
+  if (Tok.is(tok::code_completion)) {
+    cutOffParsing();
+    Actions.CodeCompleteExpression(getCurScope(),
+                                   PreferredType.get(Tok.getLocation()));
+    return ExprError();
+  }
+
+  ExprResult LHS = ParseCastExpression(
+      AnyCastExpr, /*isAddressOfOperand=*/false, NotTypeCast);
+  return ParseRHSOfBinaryExpression(LHS, prec::Conditional);
+}
+
 /// Parse an assignment expression where part of an Objective-C message
 /// send has already been parsed.
 ///
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index e6a4d3e63e4aa8..e712745a237c3e 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -303,6 +303,41 @@ static Attr *handleAlwaysInlineAttr(Sema &S, Stmt *St, const ParsedAttr &A,
   return ::new (S.Context) AlwaysInlineAttr(S.Context, A);
 }
 
+static Attr *handleAssumeAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                              SourceRange Range) {
+  if (A.getNumArgs() != 1 || !A.getArgAsExpr(0)) {
+    S.Diag(A.getLoc(), diag::err_assume_attr_args) << Range;
+    return nullptr;
+  }
+
+  if (!isa<NullStmt>(St)) {
+    S.Diag(A.getLoc(), diag::err_assume_attr_wrong_target) << Range;
+    return nullptr;
+  }
+
+  auto *Assumption = A.getArgAsExpr(0);
+  if (Assumption->getDependence() == ExprDependence::None) {
+    ExprResult Res = S.CorrectDelayedTyposInExpr(Assumption);
+    if (Res.isInvalid())
+      return nullptr;
+    Res = S.CheckPlaceholderExpr(Res.get());
+    if (Res.isInvalid())
+      return nullptr;
+    Res = S.PerformContextuallyConvertToBool(Res.get());
+    if (Res.isInvalid())
+      return nullptr;
+    Assumption = Res.get();
+
+    if (Assumption->HasSideEffects(S.Context, /*IncludePossibleEffects=*/true))
+      S.Diag(A.getLoc(), diag::warn_assume_side_effects) << A.getAttrName() << Range;
+  }
+
+  if (!S.getLangOpts().CPlusPlus23)
+    S.Diag(A.getLoc(), diag::ext_cxx23_attr) << A << Range;
+
+  return ::new (S.Context) AssumeAttr(S.Context, A, Assumption);
+}
+
 static Attr *handleMustTailAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                                 SourceRange Range) {
   // Validation is in Sema::ActOnAttributedStmt().
@@ -594,6 +629,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
   switch (A.getKind()) {
   case ParsedAttr::AT_AlwaysInline:
     return handleAlwaysInlineAttr(S, St, A, Range);
+  case ParsedAttr::AT_Assume:
+    return handleAssumeAttr(S, St, A, Range);
   case ParsedAttr::AT_FallThrough:
     return handleFallThroughAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
diff --git a/clang/test/CodeGenCXX/cxx23-assume.cpp b/clang/test/CodeGenCXX/cxx23-assume.cpp
new file mode 100644
index 00000000000000..8a8438e8edc057
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx23-assume.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++23 %s -emit-llvm -o - | FileCheck %s
+
+bool f();
+
+// CHECK: @_Z1gii(i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[X_ADDR:%.*]] = alloca i32
+// CHECK-NEXT: [[Y_ADDR:%.*]] = alloca i32
+// CHECK-NEXT: store i32 [[X]], ptr [[X_ADDR]]
+// CHECK-NEXT: store i32 [[Y]], ptr [[Y_ADDR]]
+void g(int x, int y) {
+  // Not emitted because it has side-effects.
+  [[assume(f())]];
+
+  // CHECK-NEXT: call void @llvm.assume(i1 true)
+  [[assume((1, 2))]];
+
+  // [[X1:%.*]] = load i32, ptr [[X_ADDR]]
+  // [[CMP1:%.*]] = icmp ne i32 [[X1]], 27
+  // call void @llvm.assume(i1 [[CMP1]])
+  [[assume(x != 27)]];
+
+  // [[X2:%.*]] = load i32, ptr [[X_ADDR]]
+  // [[Y2:%.*]] = load i32, ptr [[Y_ADDR]]
+  // [[CMP2:%.*]] = icmp eq i32 [[X2]], [[Y2]]
+  // call void @llvm.assume(i1 [[CMP2]])
+  [[assume(x == y)]];
+}
diff --git a/clang/test/Parser/cxx23-assume.cpp b/clang/test/Parser/cxx23-assume.cpp
new file mode 100644
index 00000000000000..2a9b8b6a248821
--- /dev/null
+++ b/clang/test/Parser/cxx23-assume.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++23 -x c++ %s -verify
+
+void f(int x, int y) {
+  [[assume(true)]];
+  [[assume(1)]];
+  [[assume(1.0)]];
+  [[assume(1 + 2 == 3)]];
+  [[assume(x ? 1 : 2)]];
+  [[assume(x && y)]];
+  [[assume(true)]] [[assume(true)]];
+
+  [[assume(x = 2)]]; // expected-error {{requires parentheses}}
+  [[assume(2, 3)]]; // expected-error {{requires parentheses}}
+}
diff --git a/clang/test/SemaCXX/cxx23-assume.cpp b/clang/test/SemaCXX/cxx23-assume.cpp
new file mode 100644
index 00000000000000..15ea831f77f10d
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-assume.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++23 -x c++ %s -verify
+// RUN: %clang_cc1 -std=c++20 -pedantic -x c++ %s -verify=ext,expected
+
+struct A{};
+struct B{ explicit operator bool() { return true; } };
+
+template <bool cond>
+void f() {
+  [[assume(cond)]]; // ext-warning {{C++23 extension}}
+}
+
+template <bool cond>
+struct S {
+  void f() {
+    [[assume(cond)]]; // ext-warning {{C++23 extension}}
+  }
+};
+
+bool f2();
+
+void g(int x) {
+  f<true>();
+  f<false>();
+  S<true>{}.f();
+  S<false>{}.f();
+  [[assume(f2())]]; // expected-warning {{side effects that will be discarded}} ext-warning {{C++23 extension}}
+
+  [[assume((x = 3))]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(x++)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(++x)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume([]{ return true; }())]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(B{})]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume((1, 2))]]; // expected-warning {{has no effect}} // ext-warning {{C++23 extension}}
+
+  [[assume]]; // expected-error {{takes one argument}}
+  [[assume(z)]]; // expected-error {{undeclared identifier}}
+  [[assume(A{})]]; // expected-error {{not contextually convertible to 'bool'}}
+  [[assume(true)]] if (true) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] for (;false;) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] while (false) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] label:; // expected-error {{cannot be applied to a declaration}}
+  [[assume(true)]] goto label; // expected-error {{only applies to empty statements}}
+}
+
+// Check that 'x' is ODR-used here.
+constexpr int h(int x) { return sizeof([=] { [[assume(x)]]; }); } // ext-warning {{C++23 extension}}
+static_assert(h(4) == sizeof(int));
+
+static_assert(__has_cpp_attribute(assume));

@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

This implements the C++23 assume attribute.

This pr currently does not include any changes to constant evaluation. Even though the standard specifies that the behaviour is undefined should an assumption evaluate to false at runtime [dcl.attr.assume], it also mentions that we’re not required to diagnose that [expr.const], as I understand it.


Full diff: https://github.com/llvm/llvm-project/pull/81014.diff

14 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/include/clang/Basic/Attr.td (+9)
  • (modified) clang/include/clang/Basic/AttrDocs.td (+24)
  • (modified) clang/include/clang/Basic/DiagnosticGroups.td (+3-1)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+3)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7)
  • (modified) clang/include/clang/Parse/Parser.h (+7)
  • (modified) clang/lib/CodeGen/CGStmt.cpp (+9-2)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+54-1)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+13)
  • (modified) clang/lib/Sema/SemaStmtAttr.cpp (+37)
  • (added) clang/test/CodeGenCXX/cxx23-assume.cpp (+28)
  • (added) clang/test/Parser/cxx23-assume.cpp (+14)
  • (added) clang/test/SemaCXX/cxx23-assume.cpp (+50)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 802c44b6c86080..6312e100913613 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -95,6 +95,7 @@ C++23 Feature Support
 
 - Implemented `P2718R0: Lifetime extension in range-based for loops <https://wg21.link/P2718R0>`_. Also
   materialize temporary object which is a prvalue in discarded-value expression.
+- Implemented `P1774R8: Portable assumptions <https://wg21.link/P1774R8>`_.
 
 C++2c Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index b2d5309e142c1a..6cd2a541da17d9 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1564,6 +1564,15 @@ def Unlikely : StmtAttr {
 }
 def : MutualExclusions<[Likely, Unlikely]>;
 
+def Assume : StmtAttr {
+  let Spellings = [CXX11<"", "assume", 202302>];
+  let Subjects = SubjectList<[NullStmt], ErrorDiag, "empty statements">;
+  // The standard only allows a conditional-expression here, but we ought
+  // to get better results by handling that in Sema.
+  let Args = [ExprArgument<"Assumption">];
+  let Documentation = [AssumeDocs];
+}
+
 def NoMerge : DeclOrStmtAttr {
   let Spellings = [Clang<"nomerge">];
   let Documentation = [NoMergeDocs];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 041786f37fb8a7..1b27a46ef1d901 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1996,6 +1996,30 @@ Here is an example:
   }];
 }
 
+def AssumeDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "assume";
+  let Content = [{
+The ``assume`` attribute is used to indicate to the optimizer that a
+certain condition can be assumed to be true at a certain point in the
+program. If this condition is violated at runtime, the behavior is
+undefined. ``assume`` can only be applied to a null statement.
+
+Note that `clang::assume` is a different attribute. Always write ``assume``
+without a namespace if you intend to use the standard C++ attribute.
+
+Example:
+
+.. code-block:: c++
+
+  int f(int x, int y) {
+    [[assume(x == 27)]];
+    [[assume(x == y)]];
+    return y + 1; // Will be optimised to `return 28`.
+  }
+  }];
+}
+
 def LikelihoodDocs : Documentation {
   let Category = DocCatStmt;
   let Heading = "likely and unlikely";
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 6765721ae7002c..192b081404a827 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1124,9 +1124,11 @@ def NonGCC : DiagGroup<"non-gcc",
 def CXX14Attrs : DiagGroup<"c++14-attribute-extensions">;
 def CXX17Attrs : DiagGroup<"c++17-attribute-extensions">;
 def CXX20Attrs : DiagGroup<"c++20-attribute-extensions">;
+def CXX23Attrs : DiagGroup<"c++23-attribute-extensions">;
 def FutureAttrs : DiagGroup<"future-attribute-extensions", [CXX14Attrs,
                                                             CXX17Attrs,
-                                                            CXX20Attrs]>;
+                                                            CXX20Attrs,
+                                                            CXX23Attrs]>;
 
 def CXX23AttrsOnLambda : DiagGroup<"c++23-lambda-attributes">;
 
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index a30ab27566ec3e..9ecfdab3617e05 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -783,6 +783,9 @@ def err_ms_property_expected_comma_or_rparen : Error<
 def err_ms_property_initializer : Error<
   "property declaration cannot have a default member initializer">;
 
+def err_assume_attr_expects_cond_expr : Error<
+  "use of this expression in an 'assume' attribute requires parentheses">;
+
 def warn_cxx20_compat_explicit_bool : Warning<
   "this expression will be parsed as explicit(bool) in C++20">,
   InGroup<CXX20Compat>, DefaultIgnore;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b4dc4feee8e63a..847168af288622 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9083,6 +9083,8 @@ def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup<CXX17Attrs>;
 def ext_cxx20_attr : Extension<
   "use of the %0 attribute is a C++20 extension">, InGroup<CXX20Attrs>;
+def ext_cxx23_attr : Extension<
+  "use of the %0 attribute is a C++23 extension">, InGroup<CXX23Attrs>;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -10149,6 +10151,11 @@ def err_fallthrough_attr_outside_switch : Error<
 def err_fallthrough_attr_invalid_placement : Error<
   "fallthrough annotation does not directly precede switch label">;
 
+def err_assume_attr_args : Error<
+  "attribute 'assume' requires a single expression argument">;
+def err_assume_attr_wrong_target : Error<
+  "'assume' attribute is only allowed on empty statements">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup<CoveredSwitchDefault>, DefaultIgnore;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..0f982dbb67b41c 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1801,6 +1801,7 @@ class Parser : public CodeCompletionHandler {
   ExprResult ParseConstraintLogicalOrExpression(bool IsTrailingRequiresClause);
   // Expr that doesn't include commas.
   ExprResult ParseAssignmentExpression(TypeCastState isTypeCast = NotTypeCast);
+  ExprResult ParseConditionalExpression();
 
   ExprResult ParseMSAsmIdentifier(llvm::SmallVectorImpl<Token> &LineToks,
                                   unsigned &NumLineToksConsumed,
@@ -2953,6 +2954,12 @@ class Parser : public CodeCompletionHandler {
                                SourceLocation ScopeLoc,
                                CachedTokens &OpenMPTokens);
 
+  /// Parse a C++23 assume() attribute. Returns true on error.
+  bool ParseAssumeAttributeArg(ParsedAttributes &Attrs,
+                               IdentifierInfo *AttrName,
+                               SourceLocation AttrNameLoc,
+                               SourceLocation *EndLoc);
+
   IdentifierInfo *TryParseCXX11AttributeIdentifier(
       SourceLocation &Loc,
       Sema::AttributeCompletion Completion = Sema::AttributeCompletion::None,
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index beff0ad9da2709..66be305550b1b9 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -721,11 +721,18 @@ void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
     case attr::AlwaysInline:
       alwaysinline = true;
       break;
-    case attr::MustTail:
+    case attr::MustTail: {
       const Stmt *Sub = S.getSubStmt();
       const ReturnStmt *R = cast<ReturnStmt>(Sub);
       musttail = cast<CallExpr>(R->getRetValue()->IgnoreParens());
-      break;
+    } break;
+    case attr::Assume: {
+      const Expr *Assumption = cast<AssumeAttr>(A)->getAssumption();
+      if (!Assumption->HasSideEffects(getContext())) {
+        llvm::Value *AssumptionVal = EvaluateExprAsBool(Assumption);
+        Builder.CreateAssumption(AssumptionVal);
+      }
+    } break;
     }
   }
   SaveAndRestore save_nomerge(InNoMergeAttributedStmt, nomerge);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 79928ddb5af599..7f2762d4fe22b9 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4528,6 +4528,54 @@ static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
   }
 }
 
+/// Parse the argument to C++23's [[assume()]] attribute.
+bool Parser::ParseAssumeAttributeArg(ParsedAttributes &Attrs,
+                                     IdentifierInfo *AttrName,
+                                     SourceLocation AttrNameLoc,
+                                     SourceLocation *EndLoc) {
+  assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");
+  BalancedDelimiterTracker T(*this, tok::l_paren);
+  T.consumeOpen();
+
+  // [dcl.attr.assume]: The expression is potentially evaluated.
+  EnterExpressionEvaluationContext Unevaluated(
+      Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+
+  TentativeParsingAction TPA(*this);
+  ExprResult Res(
+      Actions.CorrectDelayedTyposInExpr(ParseConditionalExpression()));
+  if (Res.isInvalid() || !Tok.is(tok::r_paren)) {
+    // Emit a better diagnostic if this is an otherwise valid expression that
+    // is not allowed here.
+    TPA.Revert();
+    Sema::TentativeAnalysisScope Scope(Actions);
+    Res = ParseExpression();
+    if (!Res.isInvalid()) {
+      auto *E = Res.get();
+      Diag(E->getExprLoc(), diag::err_assume_attr_expects_cond_expr)
+          << AttrName << FixItHint::CreateInsertion(E->getBeginLoc(), "(")
+          << FixItHint::CreateInsertion(PP.getLocForEndOfToken(E->getEndLoc()),
+                                        ")")
+          << E->getSourceRange();
+    }
+
+    T.consumeClose();
+    return true;
+  }
+
+  TPA.Commit();
+  ArgsUnion Assumption = Res.get();
+  auto RParen = Tok.getLocation();
+  T.consumeClose();
+  Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), nullptr,
+               SourceLocation(), &Assumption, 1, ParsedAttr::Form::CXX11());
+
+  if (EndLoc)
+    *EndLoc = RParen;
+
+  return false;
+}
+
 /// ParseCXX11AttributeArgs -- Parse a C++11 attribute-argument-clause.
 ///
 /// [C++11] attribute-argument-clause:
@@ -4596,7 +4644,12 @@ bool Parser::ParseCXX11AttributeArgs(
   if (ScopeName && (ScopeName->isStr("clang") || ScopeName->isStr("_Clang")))
     NumArgs = ParseClangAttributeArgs(AttrName, AttrNameLoc, Attrs, EndLoc,
                                       ScopeName, ScopeLoc, Form);
-  else
+  // So does C++23's assume() attribute.
+  else if (!ScopeName && AttrName->isStr("assume")) {
+    if (ParseAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc))
+      return true;
+    NumArgs = 1;
+  } else
     NumArgs = ParseAttributeArgsCommon(AttrName, AttrNameLoc, Attrs, EndLoc,
                                        ScopeName, ScopeLoc, Form);
 
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 52cebdb6f64bac..cf2a7bd026c5de 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -179,6 +179,19 @@ ExprResult Parser::ParseAssignmentExpression(TypeCastState isTypeCast) {
   return ParseRHSOfBinaryExpression(LHS, prec::Assignment);
 }
 
+ExprResult Parser::ParseConditionalExpression() {
+  if (Tok.is(tok::code_completion)) {
+    cutOffParsing();
+    Actions.CodeCompleteExpression(getCurScope(),
+                                   PreferredType.get(Tok.getLocation()));
+    return ExprError();
+  }
+
+  ExprResult LHS = ParseCastExpression(
+      AnyCastExpr, /*isAddressOfOperand=*/false, NotTypeCast);
+  return ParseRHSOfBinaryExpression(LHS, prec::Conditional);
+}
+
 /// Parse an assignment expression where part of an Objective-C message
 /// send has already been parsed.
 ///
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index e6a4d3e63e4aa8..e712745a237c3e 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -303,6 +303,41 @@ static Attr *handleAlwaysInlineAttr(Sema &S, Stmt *St, const ParsedAttr &A,
   return ::new (S.Context) AlwaysInlineAttr(S.Context, A);
 }
 
+static Attr *handleAssumeAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+                              SourceRange Range) {
+  if (A.getNumArgs() != 1 || !A.getArgAsExpr(0)) {
+    S.Diag(A.getLoc(), diag::err_assume_attr_args) << Range;
+    return nullptr;
+  }
+
+  if (!isa<NullStmt>(St)) {
+    S.Diag(A.getLoc(), diag::err_assume_attr_wrong_target) << Range;
+    return nullptr;
+  }
+
+  auto *Assumption = A.getArgAsExpr(0);
+  if (Assumption->getDependence() == ExprDependence::None) {
+    ExprResult Res = S.CorrectDelayedTyposInExpr(Assumption);
+    if (Res.isInvalid())
+      return nullptr;
+    Res = S.CheckPlaceholderExpr(Res.get());
+    if (Res.isInvalid())
+      return nullptr;
+    Res = S.PerformContextuallyConvertToBool(Res.get());
+    if (Res.isInvalid())
+      return nullptr;
+    Assumption = Res.get();
+
+    if (Assumption->HasSideEffects(S.Context, /*IncludePossibleEffects=*/true))
+      S.Diag(A.getLoc(), diag::warn_assume_side_effects) << A.getAttrName() << Range;
+  }
+
+  if (!S.getLangOpts().CPlusPlus23)
+    S.Diag(A.getLoc(), diag::ext_cxx23_attr) << A << Range;
+
+  return ::new (S.Context) AssumeAttr(S.Context, A, Assumption);
+}
+
 static Attr *handleMustTailAttr(Sema &S, Stmt *St, const ParsedAttr &A,
                                 SourceRange Range) {
   // Validation is in Sema::ActOnAttributedStmt().
@@ -594,6 +629,8 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
   switch (A.getKind()) {
   case ParsedAttr::AT_AlwaysInline:
     return handleAlwaysInlineAttr(S, St, A, Range);
+  case ParsedAttr::AT_Assume:
+    return handleAssumeAttr(S, St, A, Range);
   case ParsedAttr::AT_FallThrough:
     return handleFallThroughAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
diff --git a/clang/test/CodeGenCXX/cxx23-assume.cpp b/clang/test/CodeGenCXX/cxx23-assume.cpp
new file mode 100644
index 00000000000000..8a8438e8edc057
--- /dev/null
+++ b/clang/test/CodeGenCXX/cxx23-assume.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++23 %s -emit-llvm -o - | FileCheck %s
+
+bool f();
+
+// CHECK: @_Z1gii(i32 noundef [[X:%.*]], i32 noundef [[Y:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT: [[X_ADDR:%.*]] = alloca i32
+// CHECK-NEXT: [[Y_ADDR:%.*]] = alloca i32
+// CHECK-NEXT: store i32 [[X]], ptr [[X_ADDR]]
+// CHECK-NEXT: store i32 [[Y]], ptr [[Y_ADDR]]
+void g(int x, int y) {
+  // Not emitted because it has side-effects.
+  [[assume(f())]];
+
+  // CHECK-NEXT: call void @llvm.assume(i1 true)
+  [[assume((1, 2))]];
+
+  // [[X1:%.*]] = load i32, ptr [[X_ADDR]]
+  // [[CMP1:%.*]] = icmp ne i32 [[X1]], 27
+  // call void @llvm.assume(i1 [[CMP1]])
+  [[assume(x != 27)]];
+
+  // [[X2:%.*]] = load i32, ptr [[X_ADDR]]
+  // [[Y2:%.*]] = load i32, ptr [[Y_ADDR]]
+  // [[CMP2:%.*]] = icmp eq i32 [[X2]], [[Y2]]
+  // call void @llvm.assume(i1 [[CMP2]])
+  [[assume(x == y)]];
+}
diff --git a/clang/test/Parser/cxx23-assume.cpp b/clang/test/Parser/cxx23-assume.cpp
new file mode 100644
index 00000000000000..2a9b8b6a248821
--- /dev/null
+++ b/clang/test/Parser/cxx23-assume.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++23 -x c++ %s -verify
+
+void f(int x, int y) {
+  [[assume(true)]];
+  [[assume(1)]];
+  [[assume(1.0)]];
+  [[assume(1 + 2 == 3)]];
+  [[assume(x ? 1 : 2)]];
+  [[assume(x && y)]];
+  [[assume(true)]] [[assume(true)]];
+
+  [[assume(x = 2)]]; // expected-error {{requires parentheses}}
+  [[assume(2, 3)]]; // expected-error {{requires parentheses}}
+}
diff --git a/clang/test/SemaCXX/cxx23-assume.cpp b/clang/test/SemaCXX/cxx23-assume.cpp
new file mode 100644
index 00000000000000..15ea831f77f10d
--- /dev/null
+++ b/clang/test/SemaCXX/cxx23-assume.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++23 -x c++ %s -verify
+// RUN: %clang_cc1 -std=c++20 -pedantic -x c++ %s -verify=ext,expected
+
+struct A{};
+struct B{ explicit operator bool() { return true; } };
+
+template <bool cond>
+void f() {
+  [[assume(cond)]]; // ext-warning {{C++23 extension}}
+}
+
+template <bool cond>
+struct S {
+  void f() {
+    [[assume(cond)]]; // ext-warning {{C++23 extension}}
+  }
+};
+
+bool f2();
+
+void g(int x) {
+  f<true>();
+  f<false>();
+  S<true>{}.f();
+  S<false>{}.f();
+  [[assume(f2())]]; // expected-warning {{side effects that will be discarded}} ext-warning {{C++23 extension}}
+
+  [[assume((x = 3))]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(x++)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(++x)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume([]{ return true; }())]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume(B{})]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}}
+  [[assume((1, 2))]]; // expected-warning {{has no effect}} // ext-warning {{C++23 extension}}
+
+  [[assume]]; // expected-error {{takes one argument}}
+  [[assume(z)]]; // expected-error {{undeclared identifier}}
+  [[assume(A{})]]; // expected-error {{not contextually convertible to 'bool'}}
+  [[assume(true)]] if (true) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] for (;false;) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] while (false) {} // expected-error {{only applies to empty statements}}
+  [[assume(true)]] label:; // expected-error {{cannot be applied to a declaration}}
+  [[assume(true)]] goto label; // expected-error {{only applies to empty statements}}
+}
+
+// Check that 'x' is ODR-used here.
+constexpr int h(int x) { return sizeof([=] { [[assume(x)]]; }); } // ext-warning {{C++23 extension}}
+static_assert(h(4) == sizeof(int));
+
+static_assert(__has_cpp_attribute(assume));

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

Copy link

github-actions bot commented Feb 7, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

CI on Windows seems to have had a stroke?

FAILED: cmTC_77a79.exe
cmd.exe /C "cd . && C:\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe -E vs_link_exe --intdir=CMakeFiles\cmTC_77a79.dir --rc="C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64\rc.exe" --mt="C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64\mt.exe" --manifests  -- C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\link.exe /nologo CMakeFiles\cmTC_77a79.dir\getErrc.cpp.obj  /out:cmTC_77a79.exe /implib:cmTC_77a79.lib /pdb:cmTC_77a79.pdb /version:0.0 /machine:x64  /INCREMENTAL:NO /subsystem:console  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
MT: command "C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64\mt.exe /nologo /manifest cmTC_77a79.exe.manifest /outputresource:cmTC_77a79.exe;#1" failed (exit code 0x1f) with the following output:
mt.exe : general error c101008d: Failed to write the updated manifest to the resource of file "cmTC_77a79.exe". Operation did not complete successfully because the file contains a virus or potentially unwanted software.

Because I candidly don’t quite see how my assume code would being recognised as a virus...

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

Actually, it would seem that there were some CMake errors earlier on too

@Fznamznon
Copy link
Contributor

CI on Windows seems to have had a stroke?

Yes it does have troubles ATM. We rely on Linux CI for now until Windows is fixed. Some context on the future may be found in https://discourse.llvm.org/t/rfc-future-of-windows-pre-commit-ci/76840/1 .

@@ -4528,6 +4528,54 @@ static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
}
}

/// Parse the argument to C++23's [[assume()]] attribute.
bool Parser::ParseAssumeAttributeArg(ParsedAttributes &Attrs,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully someone more familiar with the parsing will come by, my familiarity here is limited.

@erichkeane
Copy link
Collaborator

Also note, we need to update the feature-test-macro/has_attribute-expr here.

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

Also note, we need to update the feature-test-macro/has_attribute-expr here.

I’ve already added a check for __has_cpp_attribute to one of the tests. __has_attribute(assume) already evaluates to 1 even without this pr, likely because of clang::assume, but I can add a test for that as well. I’m also not aware of a feature test macro for assume? AFAIK it’s just __has_cpp_attribute.

@cor3ntin
Copy link
Contributor

cor3ntin commented Feb 7, 2024

Thanks for working on that.

GCC does checks assumption in constant evaluation. I think we should too.

My main question is whether we think supporting assume in the front-end is going to be a net positive knowing that the backend is going to pessimize some use of the attribute

https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609

We have 2 options:

  • Wait for llvm to get improve their handling of assumption based optimizations.
  • Proceed with this work hoping this encourages work on the optimizer.

I don't have a strong opinion, but we should collectively be aware of pitfalls and
https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

We have 2 options:

  • Wait for llvm to get improve their handling of assumption based optimizations.
  • Proceed with this work hoping this encourages work on the optimizer.

The standard also mentions that __has_cpp_attribute should return 0 ‘if an implementation does not attempt to deduce any such information from assumptions’ [dcl.attr.assume], so another option would be to just have it set to 0 for the time being and just not emit anything until that’s not an issue anymore.

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

GCC does checks assumption in constant evaluation. I think we should too.

I’ll look into it in that case.

@erichkeane
Copy link
Collaborator

We have 2 options:

  • Wait for llvm to get improve their handling of assumption based optimizations.
  • Proceed with this work hoping this encourages work on the optimizer.

The standard also mentions that __has_cpp_attribute should return 0 ‘if an implementation does not attempt to deduce any such information from assumptions’ [dcl.attr.assume], so another option would be to just have it set to 0 for the time being and just not emit anything until that’s not an issue anymore.

in this patch you ARE emitting the llvm assume, so I believe we ARE trying enough with this patch to mark it. So __has_cpp_attribute needs to be properly populated.

I think the confusion/conflict between this and clang::assume needs to be figured out. These two should just be, as close as possible, spellings of the same thing.

@Sirraide
Copy link
Member Author

Sirraide commented Feb 7, 2024

GCC does checks assumption in constant evaluation. I think we should too.

I’ll look into it in that case.

Alright, this should be done now.

@Sirraide
Copy link
Member Author

Sirraide commented Mar 7, 2024

I seem to have missed a test that needs to be updated—I suppose that’s what I get for only running the Sema and Codegen tests locally instead of the entire test suite.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good to me.
You should modify cxx_status

@cor3ntin
Copy link
Contributor

cor3ntin commented Mar 8, 2024

LGTM, thanks!

@Sirraide
Copy link
Member Author

Sirraide commented Mar 8, 2024

@erichkeane @AaronBallman Do we want to figure out what to do wrt OpenMP’s assume attribute before we merge this, or do we want to decide on that after this is merged? Currently, only [[assume]] is treated as the standard attribute, and [[clang::assume]] or __attribute__((assume)) are the OpenMP one.

@alexey-bataev
Copy link
Member

[[clang::assume]] or __attribute__((assume)) are the OpenMP one

OpenMP requires only [[omp::assume]], neither [[clang::assume]] nor attribute((assume)) are OpenMP requirement.

@Sirraide
Copy link
Member Author

Sirraide commented Mar 8, 2024

[[clang::assume]] or __attribute__((assume)) are the OpenMP one

OpenMP requires only [[omp::assume]], neither [[clang::assume]] nor attribute((assume)) are OpenMP requirement.

Yeah, the weird thing is I’m not sure we support [[omp::assume]] at the moment. We should definitely fix this entire situation, but I’m not sure it needs to be part of this pr.

@alexey-bataev
Copy link
Member

[[clang::assume]] or __attribute__((assume)) are the OpenMP one

OpenMP requires only [[omp::assume]], neither [[clang::assume]] nor attribute((assume)) are OpenMP requirement.

Yeah, the weird thing is I’m not sure we support [[omp::assume]] at the moment. We should definitely fix this entire situation, but I’m not sure it needs to be part of this pr.

Can you somehow fix omp::assume at first? Say, in a separate patch?

@Sirraide
Copy link
Member Author

Sirraide commented Mar 8, 2024

[[clang::assume]] or __attribute__((assume)) are the OpenMP one

OpenMP requires only [[omp::assume]], neither [[clang::assume]] nor attribute((assume)) are OpenMP requirement.

Yeah, the weird thing is I’m not sure we support [[omp::assume]] at the moment. We should definitely fix this entire situation, but I’m not sure it needs to be part of this pr.

Can you somehow fix omp::assume at first? Say, in a separate patch?

That’s independent of the changes introduced by this pr, from what I can tell at least. So it should be possible to do that irrespective of when this gets merged, yeah.

@alexey-bataev
Copy link
Member

[[clang::assume]] or __attribute__((assume)) are the OpenMP one

OpenMP requires only [[omp::assume]], neither [[clang::assume]] nor attribute((assume)) are OpenMP requirement.

Yeah, the weird thing is I’m not sure we support [[omp::assume]] at the moment. We should definitely fix this entire situation, but I’m not sure it needs to be part of this pr.

Can you somehow fix omp::assume at first? Say, in a separate patch?

That’s independent of the changes introduced by this pr, from what I can tell at least. So it should be possible to do that irrespective of when this gets merged, yeah.

That would be great!

@Sirraide
Copy link
Member Author

Sirraide commented Mar 8, 2024

If we can come to a consensus as to what to do w/ clang::assume and __attribute__((assume)) reasonably soon, then I’ll handle [[omp::assume]] in this pr as well; otherwise, I’ll open a pr for [[omp::assume]] tomorrow or so.

@erichkeane
Copy link
Collaborator

If we can come to a consensus as to what to do w/ clang::assume and __attribute__((assume)) reasonably soon, then I’ll handle [[omp::assume]] in this pr as well; otherwise, I’ll open a pr for [[omp::assume]] tomorrow or so.

This patch is big enough as it is, as far as LOC and what effect, I would prefer we split the OMP decisions off to a separate patch and discuss that /implemetn that there.

@Sirraide
Copy link
Member Author

Sirraide commented Mar 8, 2024

Yeah, I agree; I’ll wait a bit to see if Aaron has anything to say about it and merge this otherwise; once that’s done, I’ll add [[omp::assume]] in a separate pr and open an issue to discuss what to do w/ the other spellings.

@Sirraide Sirraide merged commit 2b5f68a into llvm:main Mar 9, 2024
@Sirraide Sirraide deleted the assume branch March 9, 2024 11:07
Sirraide added a commit that referenced this pull request Mar 9, 2024
Fix a test that was added in #81014 and which caused buildbots to fail.
Only check for the ‘never produces a constant expression error’ in C++20
mode.

This fixes #84623.
Sirraide added a commit that referenced this pull request May 22, 2024
This is a followup to #81014 and #84582: Before this patch, Clang 
would accept `__attribute__((assume))` and `[[clang::assume]]` as 
nonstandard spellings for the `[[omp::assume]]` attribute; this 
resulted in a potentially very confusing name clash with C++23’s 
`[[assume]]` attribute (and GCC’s `assume` attribute with the same
semantics).

This pr replaces every usage of `__attribute__((assume))`  with 
`[[omp::assume]]` and makes `__attribute__((assume))` and 
`[[clang::assume]]` alternative spellings for C++23’s `[[assume]]`; 
this shouldn’t cause any problems due to differences in appertainment
and because almost no-one was using this variant spelling to begin
with (a use in libclc has already been changed to use a different
attribute).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants