[Clang] [Parser] Support [[omp::assume]] #84582

Sirraide · 2024-03-08T23:23:52Z

This pr implements the [[omp::assume]] spelling for the __attribute__((assume)) attribute. It does not change anything about how that attribute is handled by the rest of Clang.

This pr is not meant for figuring out what to do w/ the latter—I’ll be opening a separate pr that will update the documentation etc. to use [[omp::assume]]; we should have that discussion there; this pr is mostly meant to be self-contained (and also does not depend on or conflict with #81014).

llvmbot · 2024-03-08T23:24:12Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-openmp

Author: None (Sirraide)

Changes

This pr implements the [[omp::assume]] spelling for the __attribute__((assume)) attribute. It does not change anything about how that attribute is handled by the rest of Clang.

This pr is only not meant for figuring out what to do w/ the latter—I’ll be opening a separate pr that will update the documentation etc. to use [[omp::assume]]; we should have that discussion there; this pr is mostly meant to be self-contained (and also does not depend on or conflict with #81014).

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

4 Files Affected:

(modified) clang/include/clang/Basic/Attr.td (+1-1)
(modified) clang/lib/Basic/Attributes.cpp (+6-2)
(modified) clang/lib/Parse/ParseDeclCXX.cpp (+3-1)
(modified) clang/test/Sema/attr-assume.c (+17)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index fa191c7378dba4..c6877b61fa7f1e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4144,7 +4144,7 @@ def OMPDeclareVariant : InheritableAttr {
 }
 
 def Assumption : InheritableAttr {
-  let Spellings = [Clang<"assume">];
+  let Spellings = [Clang<"assume">, CXX11<"omp", "assume">];
   let Subjects = SubjectList<[Function, ObjCMethod]>;
   let InheritEvenIfAlreadyPresent = 1;
   let Documentation = [AssumptionDocs];
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 44a4f1890d39e1..867d241a2cf847 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -47,8 +47,12 @@ int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
   // attributes. We support those, but not through the typical attribute
   // machinery that goes through TableGen. We support this in all OpenMP modes
   // so long as double square brackets are enabled.
-  if (LangOpts.OpenMP && ScopeName == "omp")
-    return (Name == "directive" || Name == "sequence") ? 1 : 0;
+  //
+  // Other OpenMP attributes (e.g. [[omp::assume]]) are handled via the
+  // regular attribute parsing machinery.
+  if (LangOpts.OpenMP && ScopeName == "omp" &&
+      (Name == "directive" || Name == "sequence"))
+    return 1;
 
   int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
   if (res)
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 62632b2d79792e..64129d7eaface5 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4579,7 +4579,9 @@ bool Parser::ParseCXX11AttributeArgs(
     return true;
   }
 
-  if (ScopeName && ScopeName->isStr("omp")) {
+  // [[omp::directive]] and [[omp::sequence]] need special handling.
+  if (ScopeName && ScopeName->isStr("omp") &&
+      (AttrName->isStr("directive") || AttrName->isStr("sequence"))) {
     Diag(AttrNameLoc, getLangOpts().OpenMP >= 51
                           ? diag::warn_omp51_compat_attributes
                           : diag::ext_omp_attributes);
diff --git a/clang/test/Sema/attr-assume.c b/clang/test/Sema/attr-assume.c
index 98deffa3a74609..3b7721647dee60 100644
--- a/clang/test/Sema/attr-assume.c
+++ b/clang/test/Sema/attr-assume.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fopenmp -DCXX -verify %s
 
+#ifndef CXX
 void f1(void) __attribute__((assume(3))); // expected-error {{expected string literal as argument of 'assume' attribute}}
 void f2(void) __attribute__((assume(int))); // expected-error {{expected string literal as argument of 'assume' attribute}}
 void f3(void) __attribute__((assume(for))); // expected-error {{expected string literal as argument of 'assume' attribute}}
@@ -12,3 +14,18 @@ void f9(void) __attribute__((assume("omp_no_openmp", "omp_no_openmp"))); // expe
 
 int g1 __attribute__((assume(0))); // expected-error {{expected string literal as argument of 'assume' attribute}}
 int g2 __attribute__((assume("omp_no_openmp"))); // expected-warning {{'assume' attribute only applies to functions and Objective-C methods}}
+
+#else
+[[omp::assume(3)]] void f1(); // expected-error {{expected string literal as argument of 'assume' attribute}}
+[[omp::assume(int)]] void f2(); // expected-error {{expected string literal as argument of 'assume' attribute}}
+[[omp::assume(for)]] void f3(); // expected-error {{expected string literal as argument of 'assume' attribute}}
+[[omp::assume("QQQQ")]] void f4(); // expected-warning {{unknown assumption string 'QQQQ'; attribute is potentially ignored}}
+[[omp::assume("omp_no_openmp")]] void f5();
+[[omp::assume("omp_noopenmp")]] void f6(); // expected-warning {{unknown assumption string 'omp_noopenmp' may be misspelled; attribute is potentially ignored, did you mean 'omp_no_openmp'?}}
+[[omp::assume("omp_no_openmp_routine")]] void f7(); // expected-warning {{unknown assumption string 'omp_no_openmp_routine' may be misspelled; attribute is potentially ignored, did you mean 'omp_no_openmp_routines'?}}
+[[omp::assume("omp_no_openmp1")]] void f8(); // expected-warning {{unknown assumption string 'omp_no_openmp1' may be misspelled; attribute is potentially ignored, did you mean 'omp_no_openmp'?}}
+[[omp::assume("omp_no_openmp", "omp_no_openmp")]] void f9(); // expected-error {{'assume' attribute takes one argument}}
+
+[[omp::assume(3)]] int g1; // expected-error {{expected string literal as argument of 'assume' attribute}}
+[[omp::assume("omp_no_openmp")]] int g2; // expected-warning {{'assume' attribute only applies to functions and Objective-C methods}}
+#endif

Sirraide · 2024-03-08T23:26:37Z

@alexey-bataev I have a few more questions about [[omp::assume]]:

Is this supported in C? Because the [[]] spelling is only officially available in C23 and later (we do support it in earlier language modes, but users will get a warning if -pedantic is passed)
Is there any place where I can find some more documentation about this attribute? I tried to search for it online, but I couldn’t find anything about it.

Sirraide · 2024-03-08T23:30:34Z

One more thing: this is just a different spelling for what was previously already exposed as __attribute__((assume)); I’m not sure if/how familiar you are with that one, but I hope its semantics align with what [[omp::assume]] is supposed to do. I’m asking because I’m not terribly familiar with OpenMP myself.

alexey-bataev · 2024-03-08T23:39:51Z

@alexey-bataev I have a few more questions about [[omp::assume]]:

Is this supported in C? Because the [[]] spelling is only officially available in C23 and later (we do support it in earlier language modes, but users will get a warning if -pedantic is passed)

Is there any place where I can find some more documentation about this attribute? I tried to search for it online, but I couldn’t find anything about it.

Currently only for C++11 and higher

alexey-bataev · 2024-03-08T23:40:32Z

@alexey-bataev I have a few more questions about [[omp::assume]]:

Is this supported in C? Because the [[]] spelling is only officially available in C23 and later (we do support it in earlier language modes, but users will get a warning if -pedantic is passed)

Is there any place where I can find some more documentation about this attribute? I tried to search for it online, but I couldn’t find anything about it.

https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5-2.pdf, pages 49-50

Sirraide · 2024-03-08T23:43:19Z

Currently only for C++11 and higher

Ok, good; that’ll make this easier.

https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5-2.pdf, pages 49-50

Thanks. At a glance, it looks like there are more things that also still need to be implemented in the frontend, but adding [[omp::assume]] will allow us to deprecate/remove the __attribute__((assume)) spelling that’s causing problems for us at the moment, so that’s a more pressing concern right now.

alexey-bataev · 2024-03-08T23:43:51Z

One more thing: this is just a different spelling for what was previously already exposed as __attribute__((assume)); I’m not sure if/how familiar you are with that one, but I hope its semantics align with what [[omp::assume]] is supposed to do. I’m asking because I’m not terribly familiar with OpenMP myself.

Some details about assume directive

https://www.openmp.org/wp-content/uploads/OpenMP-assume_Doerfert.pdf

Sirraide · 2024-03-08T23:51:15Z

Ok yeah, good, looks like that’s the right attribute to map [[omp::assume]] to.

clang/test/Sema/attr-assume.c

AaronBallman

The precommit CI failure is not an actual issue, so LGTM with a suggestion for the release note. Thank you for this!

clang/docs/ReleaseNotes.rst

Sirraide · 2024-03-12T12:44:13Z

As I mentioned, I’m going to make a follow-up pr to this soon so we can discuss what to do with __attribute__((assume))/[[clang::assume]].

Sirraide · 2024-03-12T15:43:06Z

As I mentioned, I’m going to make a follow-up pr to this soon so we can discuss what to do with __attribute__((assume))/[[clang::assume]].

Just opened a pr for this: #84934

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).

[Clang] [Parser] Support [[omp::assume]]

d436f7f

Sirraide added openmp clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 8, 2024

Sirraide requested review from AaronBallman and erichkeane March 8, 2024 23:23

llvmbot added the clang Clang issues not falling into any other category label Mar 8, 2024

Sirraide requested review from alexey-bataev and jdoerfert March 8, 2024 23:34

Sirraide added 3 commits March 9, 2024 01:02

[Clang] Add release note

5096f85

Merge branch 'main' into omp-assume-1

9951dfb

Merge branch 'main' into omp-assume-1

6f8dfd5

alexey-bataev reviewed Mar 11, 2024

View reviewed changes

clang/test/Sema/attr-assume.c Outdated Show resolved Hide resolved

[NFC] Move [[omp::assume]] tests to test/OpenMP

4338314

llvmbot added the clang:openmp OpenMP related changes to Clang label Mar 11, 2024

AaronBallman approved these changes Mar 12, 2024

View reviewed changes

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

[Clang] Add openmp section to release notes

3d41de0

Sirraide merged commit b5a16b6 into llvm:main Mar 12, 2024

Sirraide deleted the omp-assume-1 branch March 12, 2024 12:42

Sirraide mentioned this pull request Mar 12, 2024

[Clang] __attribute__((assume)) refactor #84934

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] [Parser] Support [[omp::assume]] #84582

[Clang] [Parser] Support [[omp::assume]] #84582

Uh oh!

Sirraide commented Mar 8, 2024 •

edited

Loading

Uh oh!

llvmbot commented Mar 8, 2024 •

edited

Loading

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024 •

edited

Loading

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

Uh oh!

AaronBallman left a comment

Uh oh!

Uh oh!

Sirraide commented Mar 12, 2024 •

edited

Loading

Uh oh!

Sirraide commented Mar 12, 2024

Uh oh!

Uh oh!

[Clang] [Parser] Support [[omp::assume]] #84582

[Clang] [Parser] Support [[omp::assume]] #84582

Uh oh!

Conversation

Sirraide commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

alexey-bataev commented Mar 8, 2024

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Sirraide commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sirraide commented Mar 12, 2024

Uh oh!

Uh oh!

Sirraide commented Mar 8, 2024 •

edited

Loading

llvmbot commented Mar 8, 2024 •

edited

Loading

Sirraide commented Mar 8, 2024 •

edited

Loading

Sirraide commented Mar 12, 2024 •

edited

Loading