[OpenMP][C++] Allow #pragma omp simd in constexpr functions

[jdoerfert]

Summary

There is no reason (obvious to me) that would prevent us from ignoring
the "simd" part during constexpr eval, assuming there are no clauses.
The potentially controversial part is the change from Equal to Encloses
when we do the context lookup.

Fixes https://github.com/llvm/llvm-project/issues/58092

[aaron.ballman]

Adding a few more reviewers who have been poking around in constant expression evaluation lately.

clang/include/clang/Basic/DiagnosticSemaKinds.td

2722–2723

You should manually re-flow this to the usual 80-col limit.

clang/lib/AST/ExprConstant.cpp

8274–8279

This comment no longer matches the code. It also suggests that the way to address your issue is to capture the variable into the current frame -- did you explore that approach?

clang/lib/Sema/SemaDeclCXX.cpp

2175–2178

C++20 now has constexpr destructors, but there are also other kinds of special member functions too. It seems like the constraint really is "constant evaluation of a function with SIMD clauses is invalid" and so we might want to go with a more generic diagnostic instead of trying to distinguish between kinds of functions.

[jdoerfert]

clang/lib/AST/ExprConstant.cpp

8274–8279

The problem is that OpenMP introduces artificial capture statements. I am not sure if/how those can be "merged" into the parent context and in general that is not allowed (e.g., as we allow clauses on SIMD, like private). I feel the comment needs updating and I'll try that.

clang/lib/Sema/SemaDeclCXX.cpp

2175–2178

I copied this from below (see comment). I would assume the two have to stay in sync. All the differentiation of constructor does it to specify where it was found, no? So:
SIMD with clauses is not supported, found in XYZ.

2202

^ here

[aaron.ballman]

clang/lib/AST/ExprConstant.cpp

8274–8279

Is there a way to limit the change to only OpenMP captured variables so that we're not changing the semantics for all language modes?

clang/lib/Sema/SemaDeclCXX.cpp

2175–2178

Yeah, they should stay in sync; I don't insist on a change to the diagnostic as part of this review unless you feel like it.

[jdoerfert]

clang/lib/AST/ExprConstant.cpp

8274–8279

I could reasonably guard this to look for -fopenmp if we want to. The variables themselves are probably not easy to identify. I'll look at it again next week in the hope to make more sense of the intermediate frame and maybe avoid this to spill over to other models.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_cxx17_compat_constexpr_body_invalid_stmt : Warning<
   InGroup<CXXPre20Compat>, DefaultIgnore;
 def ext_constexpr_body_invalid_stmt_cxx2b : ExtWarn<
   "use of this statement in a constexpr %select{function|constructor}0 "
   "is a C++2b extension">, InGroup<CXX2b>;
 def warn_cxx20_compat_constexpr_body_invalid_stmt : Warning<
   "use of this statement in a constexpr %select{function|constructor}0 "
   "is incompatible with C++ standards before C++2b">,
   InGroup<CXXPre2bCompat>, DefaultIgnore;
+def err_constexpr_body_invalid_omp_simd_stmt_with_clauses : Error<
+  "OpenMP simd statement with clauses not allowed in %select{constexpr|consteval}1 %select{function|constructor}0">;
 def ext_constexpr_type_definition : ExtWarn<
   "type definition in a constexpr %select{function|constructor}0 "
   "is a C++14 extension">, InGroup<CXX14>;
 def warn_cxx11_compat_constexpr_type_definition : Warning<
   "type definition in a constexpr %select{function|constructor}0 "
   "is incompatible with C++ standards before C++14">,
   InGroup<CXXPre14Compat>, DefaultIgnore;
 def err_constexpr_vla : Error<

clang/lib/AST/ExprConstant.cpp

return EvaluateStmt(Result, Info, cast<AttributedStmt>(S)->getSubStmt(),
                         Case);
 
   case Stmt::CaseStmtClass:
   case Stmt::DefaultStmtClass:
     return EvaluateStmt(Result, Info, cast<SwitchCase>(S)->getSubStmt(), Case);
   case Stmt::CXXTryStmtClass:
     // Evaluate try blocks by evaluating all sub statements.
     return EvaluateStmt(Result, Info, cast<CXXTryStmt>(S)->getTryBlock(), Case);
+
+  case Stmt::OMPSimdDirectiveClass: {
+    const Stmt *LoopStmt = cast<OMPSimdDirective>(S)->getAssociatedStmt();
+    if (auto *CS = dyn_cast<CapturedStmt>(LoopStmt))
+      LoopStmt = CS->getCapturedStmt();
+    return EvaluateStmt(Result, Info, LoopStmt, Case);
+  }
   }
 }
 
 /// CheckTrivialDefaultConstructor - Check whether a constructor is a trivial
 /// default constructor. If so, we'll fold it whether or not it's marked as
 /// constexpr. If it is marked as constexpr, we will never implicitly define it,
 /// so we need special handling.
 static bool CheckTrivialDefaultConstructor(EvalInfo &Info, SourceLocation Loc,
if (auto *FD = Info.CurrentCall->LambdaCaptureFields.lookup(VD)) {
       }
       return true;
     }
   }
 
   CallStackFrame *Frame = nullptr;
   unsigned Version = 0;
   if (VD->hasLocalStorage()) {
     // Only if a local variable was declared in the function currently being
     // evaluated, do we expect to be able to find its value in the current
     // frame. (Otherwise it was likely declared in an enclosing context and
     // could either have a valid evaluatable value (for e.g. a constexpr
     // variable) or be ill-formed (and trigger an appropriate evaluation
     // diagnostic)).
     CallStackFrame *CurrFrame = Info.CurrentCall;
-    if (CurrFrame->Callee && CurrFrame->Callee->Equals(VD->getDeclContext())) {
+    if (CurrFrame->Callee &&
+        CurrFrame->Callee->Encloses(VD->getDeclContext())) {
       // Function parameters are stored in some caller's frame. (Usually the
       // immediate caller, but for an inherited constructor they may be more
       // distant.)
       if (auto *PVD = dyn_cast<ParmVarDecl>(VD)) {
         if (CurrFrame->Arguments) {
           VD = CurrFrame->Arguments.getOrigParam(PVD);
           Frame =
               Info.getCallFrameAndDepth(CurrFrame->Arguments.CallIndex).first;

clang/lib/Sema/SemaDeclCXX.cpp

case Stmt::CXXCatchStmtClass:
     // Do not bother checking the language mode (already covered by the
     // try block check).
     if (!CheckConstexprFunctionStmt(
             SemaRef, Dcl, cast<CXXCatchStmt>(S)->getHandlerBlock(), ReturnStmts,
             Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind))
       return false;
     return true;
 
+  case Stmt::OMPSimdDirectiveClass: {
+    if (!cast<OMPSimdDirective>(S)->clauses().empty()) {
+      if (Kind == Sema::CheckConstexprKind::Diagnose) {
+        SemaRef.Diag(
+            S->getBeginLoc(),
+            diag::err_constexpr_body_invalid_omp_simd_stmt_with_clauses)
+            << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval();
+      }
+      return false;
+    }
+    Stmt *LoopStmt = cast<OMPSimdDirective>(S)->getAssociatedStmt();
+    if (auto *CS = dyn_cast<CapturedStmt>(LoopStmt))
+      LoopStmt = CS->getCapturedStmt();
+    return CheckConstexprFunctionStmt(SemaRef, Dcl, LoopStmt, ReturnStmts,
+                                      Cxx1yLoc, Cxx2aLoc, Cxx2bLoc, Kind);
+  }
+
   default:
     if (!isa<Expr>(S))
       break;
 
     // C++1y allows expression-statements.
     if (!Cxx1yLoc.isValid())
       Cxx1yLoc = S->getBeginLoc();
     return true;
   }
 
   if (Kind == Sema::CheckConstexprKind::Diagnose) {
     SemaRef.Diag(S->getBeginLoc(), diag::err_constexpr_body_invalid_stmt)
         << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval();
   }
   return false;
 }
 
 /// Check the body for the given constexpr function declaration only contains
 /// the permitted types of statement. C++11 [dcl.constexpr]p3,p4.
 ///
 /// \return true if the body is OK, false if we have found or diagnosed a
 /// problem.

clang/test/SemaCXX/constexpr-openmp-simd.cpp

+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -fopenmp -triple x86_64-apple-macosx10.14.0 %s
+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -fopenmp -triple x86_64-apple-macosx10.14.0 %s -fno-signed-char
+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -fopenmp -triple aarch64_be-linux-gnu %s
+// RUN: %clang_cc1 -emit-llvm -std=c++2a -fopenmp -triple aarch64_be-linux-gnu %s -DEMIT_IR -o - | FileCheck %s
+
+constexpr int good(int r) {
+
+  #pragma omp simd
+  for (int i = 0; i != 10; ++i)
+    r += 1;
+
+  return r;
+}
+
+int test1() {
+  if (good(10) == 20)
+    return 42;
+  return good(7);
+// Make sure this is folded to 42 and not 17.
+// CHECK: ret i32 42
+}
+
+#ifndef EMIT_IR
+constexpr int bad(int r) {
+  #pragma omp simd private(r) // expected-error {{OpenMP simd statement with clauses not allowed in constexpr function}}
+  for (int i = 0; i != 10; ++i)
+    r += 1;
+
+  return r;
+}
+
+int test2() {
+  return bad(10);
+}
+#endif