[Clang] __attribute__((assume)) refactor
#84934
Conversation
Sirraide
added
openmp
clang:frontend
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-openmp Author: None (Sirraide) Changes@AaronBallman, @erichkeane This is a followup to #81014 and #84582. For anyone not familiar with what this is about: Clang provides Just to see how much would be affected by this, I’ve gone ahead and removed every last mention of
TODO:
Patch is 42.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/84934.diff 25 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 080340669b60a0..39fccc720bc9cd 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1581,10 +1581,11 @@ def Unlikely : StmtAttr {
def : MutualExclusions<[Likely, Unlikely]>;
def CXXAssume : StmtAttr {
- let Spellings = [CXX11<"", "assume", 202207>];
+ let Spellings = [CXX11<"", "assume", 202207>, Clang<"assume">];
let Subjects = SubjectList<[NullStmt], ErrorDiag, "empty statements">;
let Args = [ExprArgument<"Assumption">];
let Documentation = [CXXAssumeDocs];
+ let HasCustomParsing = 1;
}
def NoMerge : DeclOrStmtAttr {
@@ -4159,7 +4160,7 @@ def OMPDeclareVariant : InheritableAttr {
}
def OMPAssume : InheritableAttr {
- let Spellings = [Clang<"assume">, CXX11<"omp", "assume">];
+ let Spellings = [CXX11<"omp", "assume">, Clang<"omp_assume">];
let Subjects = SubjectList<[Function, ObjCMethod]>;
let InheritEvenIfAlreadyPresent = 1;
let Documentation = [OMPAssumeDocs];
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 2c07cd09b0d5b7..855d57228c56fc 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4661,7 +4661,7 @@ def OMPAssumeDocs : Documentation {
let Category = DocCatFunction;
let Heading = "assume";
let Content = [{
-Clang supports the ``__attribute__((assume("assumption")))`` attribute to
+Clang supports the ``[[omp::assume("assumption")]]`` attribute to
provide additional information to the optimizer. The string-literal, here
"assumption", will be attached to the function declaration such that later
analysis and optimization passes can assume the "assumption" to hold.
@@ -4673,7 +4673,7 @@ A function can have multiple assume attributes and they propagate from prior
declarations to later definitions. Multiple assumptions are aggregated into a
single comma separated string. Thus, one can provide multiple assumptions via
a comma separated string, i.a.,
-``__attribute__((assume("assumption1,assumption2")))``.
+``[[omp::assume("assumption1,assumption2")]]``.
While LLVM plugins might provide more assumption strings, the default LLVM
optimization passes are aware of the following assumptions:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c54105507753eb..5b95b0f11d41cd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10171,9 +10171,6 @@ 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 '%0' requires a single expression argument">;
-
def warn_unreachable_default : Warning<
"default label in switch which covers all enumeration values">,
InGroup<CoveredSwitchDefault>, DefaultIgnore;
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index dd179414a14191..3423a1d7e22430 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -629,6 +629,9 @@ void Parser::ParseGNUAttributeArgs(
ParseAttributeWithTypeArg(*AttrName, AttrNameLoc, Attrs, ScopeName,
ScopeLoc, Form);
return;
+ } else if (AttrKind == ParsedAttr::AT_CXXAssume) {
+ ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc);
+ return;
}
// These may refer to the function arguments, but need to be parsed early to
@@ -683,6 +686,10 @@ unsigned Parser::ParseClangAttributeArgs(
ParseTypeTagForDatatypeAttribute(*AttrName, AttrNameLoc, Attrs, EndLoc,
ScopeName, ScopeLoc, Form);
break;
+
+ case ParsedAttr::AT_CXXAssume:
+ ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc);
+ break;
}
return !Attrs.empty() ? Attrs.begin()->getNumArgs() : 0;
}
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 691857e88beb49..c72af6a24033d2 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -656,7 +656,8 @@ bool Sema::CheckRebuiltStmtAttributes(ArrayRef<const Attr *> Attrs) {
ExprResult Sema::ActOnCXXAssumeAttr(Stmt *St, const ParsedAttr &A,
SourceRange Range) {
if (A.getNumArgs() != 1 || !A.getArgAsExpr(0)) {
- Diag(A.getLoc(), diag::err_assume_attr_args) << A.getAttrName() << Range;
+ Diag(A.getLoc(), diag::err_attribute_wrong_number_arguments)
+ << A.getAttrName() << 1 << Range;
return ExprError();
}
diff --git a/clang/test/CodeGen/assume_attr.c b/clang/test/CodeGen/assume_attr.c
deleted file mode 100644
index 338a625188af08..00000000000000
--- a/clang/test/CodeGen/assume_attr.c
+++ /dev/null
@@ -1,58 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm -triple i386-linux-gnu %s -o - | FileCheck %s
-// RUN: %clang_cc1 -x c -emit-pch -o %t %s
-// RUN: %clang_cc1 -include-pch %t %s -emit-llvm -o - | FileCheck %s
-
-// TODO: for "foo" and "bar", "after" is not added as it appears "after" the first use or definition respectively. There might be a way to allow that.
-
-// CHECK: define{{.*}} void @bar() #0
-// CHECK: define{{.*}} void @baz() #1
-// CHECK: declare{{.*}} void @foo() #2
-// CHECK: attributes #0
-// CHECK-SAME: "llvm.assume"="bar:before1,bar:before2,bar:before3,bar:def1,bar:def2"
-// CHECK: attributes #1
-// CHECK-SAME: "llvm.assume"="baz:before1,baz:before2,baz:before3,baz:def1,baz:def2,baz:after"
-// CHECK: attributes #2
-// CHECK-SAME: "llvm.assume"="foo:before1,foo:before2,foo:before3"
-
-#ifndef HEADER
-#define HEADER
-
-/// foo: declarations only
-
-__attribute__((assume("foo:before1"))) void foo(void);
-
-__attribute__((assume("foo:before2")))
-__attribute__((assume("foo:before3"))) void
-foo(void);
-
-/// baz: static function declarations and a definition
-
-__attribute__((assume("baz:before1"))) static void baz(void);
-
-__attribute__((assume("baz:before2")))
-__attribute__((assume("baz:before3"))) static void
-baz(void);
-
-// Definition
-__attribute__((assume("baz:def1,baz:def2"))) static void baz(void) { foo(); }
-
-__attribute__((assume("baz:after"))) static void baz(void);
-
-/// bar: external function declarations and a definition
-
-__attribute__((assume("bar:before1"))) void bar(void);
-
-__attribute__((assume("bar:before2")))
-__attribute__((assume("bar:before3"))) void
-bar(void);
-
-// Definition
-__attribute__((assume("bar:def1,bar:def2"))) void bar(void) { baz(); }
-
-__attribute__((assume("bar:after"))) void bar(void);
-
-/// back to foo
-
-__attribute__((assume("foo:after"))) void foo(void);
-
-#endif
diff --git a/clang/test/CodeGenCXX/assume_attr.cpp b/clang/test/CodeGenCXX/assume_attr.cpp
index dbe76501377c0a..962dcc470f676a 100644
--- a/clang/test/CodeGenCXX/assume_attr.cpp
+++ b/clang/test/CodeGenCXX/assume_attr.cpp
@@ -8,77 +8,77 @@
/// foo: declarations only
-__attribute__((assume("foo:before1"))) void foo();
+[[omp::assume("foo:before1")]] void foo();
-__attribute__((assume("foo:before2")))
-__attribute__((assume("foo:before3"))) void
+[[omp::assume("foo:before2")]]
+[[omp::assume("foo:before3")]] void
foo();
/// baz: static function declarations and a definition
-__attribute__((assume("baz:before1"))) static void baz();
+[[omp::assume("baz:before1")]] static void baz();
-__attribute__((assume("baz:before2")))
-__attribute__((assume("baz:before3"))) static void
+[[omp::assume("baz:before2")]]
+[[omp::assume("baz:before3")]] static void
baz();
// Definition
-__attribute__((assume("baz:def1,baz:def2"))) static void baz() { foo(); }
+[[omp::assume("baz:def1,baz:def2")]] static void baz() { foo(); }
-__attribute__((assume("baz:after"))) static void baz();
+[[omp::assume("baz:after")]] static void baz();
/// bar: external function declarations and a definition
-__attribute__((assume("bar:before1"))) void bar();
+[[omp::assume("bar:before1")]] void bar();
-__attribute__((assume("bar:before2")))
-__attribute__((assume("bar:before3"))) void
+[[omp::assume("bar:before2")]]
+[[omp::assume("bar:before3")]] void
bar();
// Definition
-__attribute__((assume("bar:def1,bar:def2"))) void bar() { baz(); }
+[[omp::assume("bar:def1,bar:def2")]] void bar() { baz(); }
-__attribute__((assume("bar:after"))) void bar();
+[[omp::assume("bar:after")]] void bar();
/// back to foo
-__attribute__((assume("foo:after"))) void foo();
+[[omp::assume("foo:after")]] void foo();
/// class tests
class C {
- __attribute__((assume("C:private_method"))) void private_method();
- __attribute__((assume("C:private_static"))) static void private_static();
+ [[omp::assume("C:private_method")]] void private_method();
+ [[omp::assume("C:private_static")]] static void private_static();
public:
- __attribute__((assume("C:public_method1"))) void public_method();
- __attribute__((assume("C:public_static1"))) static void public_static();
+ [[omp::assume("C:public_method1")]] void public_method();
+ [[omp::assume("C:public_static1")]] static void public_static();
};
-__attribute__((assume("C:public_method2"))) void C::public_method() {
+[[omp::assume("C:public_method2")]] void C::public_method() {
private_method();
}
-__attribute__((assume("C:public_static2"))) void C::public_static() {
+[[omp::assume("C:public_static2")]] void C::public_static() {
private_static();
}
/// template tests
template <typename T>
-__attribute__((assume("template_func<T>"))) void template_func() {}
+[[omp::assume("template_func<T>")]] void template_func() {}
template <>
-__attribute__((assume("template_func<float>"))) void template_func<float>() {}
+[[omp::assume("template_func<float>")]] void template_func<float>() {}
template <>
void template_func<int>() {}
template <typename T>
struct S {
- __attribute__((assume("S<T>::method"))) void method();
+ [[omp::assume("S<T>::method")]] void method();
};
template <>
-__attribute__((assume("S<float>::method"))) void S<float>::method() {}
+[[omp::assume("S<float>::method")]] void S<float>::method() {}
template <>
void S<int>::method() {}
diff --git a/clang/test/OpenMP/assumes_codegen.cpp b/clang/test/OpenMP/assumes_codegen.cpp
index 6a5871c303aade..2682e394d0d324 100644
--- a/clang/test/OpenMP/assumes_codegen.cpp
+++ b/clang/test/OpenMP/assumes_codegen.cpp
@@ -67,46 +67,46 @@ int lambda_outer() {
}
#pragma omp end assumes
-// AST: __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void foo() {
-// AST-NEXT: }
-// AST-NEXT: class BAR {
-// AST-NEXT: public:
-// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAR() {
-// AST-NEXT: }
-// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void bar1() {
-// AST-NEXT: }
-// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) static void bar2() {
-// AST-NEXT: }
-// AST-NEXT: };
-// AST-NEXT: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void bar() {
-// AST-NEXT: BAR b;
-// AST-NEXT: }
-// AST-NEXT: void baz() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
-// AST-NEXT: template <typename T> class BAZ {
-// AST-NEXT: public:
-// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAZ<T>() {
-// AST-NEXT: }
-// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void baz1() {
-// AST-NEXT: }
-// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) static void baz2() {
-// AST-NEXT: }
-// AST-NEXT: };
-// AST-NEXT: template<> class BAZ<float> {
-// AST-NEXT: public:
-// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) BAZ() {
-// AST-NEXT: }
-// AST-NEXT: void baz1() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
-// AST-NEXT: static void baz2() __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp")));
-// AST-NEXT: };
-// AST-NEXT: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) void baz() {
-// AST-NEXT: BAZ<float> b;
-// AST-NEXT: }
-// AST-NEXT: __attribute__((assume("ompx_lambda_assumption"))) __attribute__((assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses"))) __attribute__((assume("omp_no_openmp"))) int lambda_outer() {
-// AST-NEXT: auto lambda_inner = []() {
-// AST-NEXT: return 42;
-// AST-NEXT: };
-// AST-NEXT: return lambda_inner();
-// AST-NEXT: }
+// AST{LITERAL}: void foo() [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: class BAR {
+// AST-NEXT{LITERAL}: public:
+// AST-NEXT{LITERAL}: BAR() [[omp::assume("ompx_range_bar_only")]] [[omp::assume("ompx_range_bar_only_2")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: void bar1() [[omp::assume("ompx_range_bar_only")]] [[omp::assume("ompx_range_bar_only_2")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: static void bar2() [[omp::assume("ompx_range_bar_only")]] [[omp::assume("ompx_range_bar_only_2")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: };
+// AST-NEXT{LITERAL}: void bar() [[omp::assume("ompx_range_bar_only")]] [[omp::assume("ompx_range_bar_only_2")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: BAR b;
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: void baz() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]];
+// AST-NEXT{LITERAL}: template <typename T> class BAZ {
+// AST-NEXT{LITERAL}: public:
+// AST-NEXT{LITERAL}: BAZ<T>() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: void baz1() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: static void baz2() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: };
+// AST-NEXT{LITERAL}: template<> class BAZ<float> {
+// AST-NEXT{LITERAL}: public:
+// AST-NEXT{LITERAL}: BAZ() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: void baz1() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]];
+// AST-NEXT{LITERAL}: static void baz2() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]];
+// AST-NEXT{LITERAL}: };
+// AST-NEXT{LITERAL}: void baz() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: BAZ<float> b;
+// AST-NEXT{LITERAL}: }
+// AST-NEXT{LITERAL}: int lambda_outer() [[omp::assume("ompx_lambda_assumption")]] [[omp::assume("omp_no_openmp_routines,ompx_another_warning,ompx_after_invalid_clauses")]] [[omp::assume("omp_no_openmp")]] {
+// AST-NEXT{LITERAL}: auto lambda_inner = []() {
+// AST-NEXT{LITERAL}: return 42;
+// AST-NEXT{LITERAL}: };
+// AST-NEXT{LITERAL}: return lambda_inner();
+// AST-NEXT{LITERAL}: }
#endif
diff --git a/clang/test/OpenMP/assumes_print.cpp b/clang/test/OpenMP/assumes_print.cpp
index a7f04edb3b1af4..6f83912a277d29 100644
--- a/clang/test/OpenMP/assumes_print.cpp
+++ b/clang/test/OpenMP/assumes_print.cpp
@@ -37,8 +37,8 @@ void baz() {
}
#pragma omp end assumes
-// CHECK: __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void foo()
-// CHECK: __attribute__((assume("ompx_range_bar_only"))) __attribute__((assume("ompx_range_bar_only_2"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void bar()
-// CHECK: __attribute__((assume("ompx_1234"))) __attribute__((assume("omp_no_openmp_routines"))) __attribute__((assume("omp_no_openmp"))) void baz()
+// CHECK{LITERAL}: void foo() [[omp::assume("omp_no_openmp_routines")]] [[omp::assume("omp_no_openmp")]]
+// CHECK{LITERAL}: void bar() [[omp::assume("ompx_range_bar_only")]] [[omp::assume("ompx_range_bar_only_2")]] [[omp::assume("omp_no_openmp_routines")]] [[omp::assume("omp_no_openmp")]]
+// CHECK{LITERAL}: void baz() [[omp::assume("ompx_1234")]] [[omp::assume("omp_no_openmp_routines")]] [[omp::assume("omp_no_openmp")]]
#endif
diff --git a/clang/test/OpenMP/assumes_template_print.cpp b/clang/test/OpenMP/assumes_template_print.cpp
index bd1100fbefffc6..b1f3df7cb6fc6a 100644
--- a/clang/test/OpenMP/assumes_template_print.cpp
+++ b/clang/test/OpenMP/assumes_template_print.cpp
@@ -17,7 +17,7 @@ template <typename T>
struct S {
int a;
// CHECK: template <typename T> struct S {
-// CHECK: __attribute__((assume("ompx_global_...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Hi @Sirraide : I appreciate the patch! However, I'm not particularly likely to get a chance to take a look before I go to the WG21 meeting next week, so I'll likely need to review this in April when I get back. I just wanted to mention that so you don't think I'm ignoring you, just away. |
Sure, no problem. |
|
List of issues that may be fixed by this: |
|
Have you had a chat with OpenMP folks? The logic would be that openmp would use something like And we should be careful about breaks, hence why I prefer a deprecation approach. |
@alexey-bataev has been commenting on this matter since the original assume pr, and according to them,
I don’t like
It wouldn’t really change meaning as there is still a difference in appertainment between the C++ attribute and the OpenMP attribute—unless you mean that it would change from just working to issuing a warning/error when applied to a function, in which case, yeah, we might want to deprecate it first. The only reason I’m considering removal as a possibility is because I don’t think anyone is actually using this spelling, from what I can tell—but of course, there may be users that we’re just not aware of... |
Specifically, libclc/generic/include/clc/clcfunc.h. |
There was a problem hiding this comment.
Thank you for working on this, it's definitely a complicated situation!
However, libclc appears to be using attribute((assume)) internally, specifically, in one header that defines a macro that is then used throughout the codebase. I’m not familiar with libclc or OpenCL, so I’ve added omp_assume as an alternative spelling for the OpenMP attribute and replaced the attribute in the header in question with attribute((omp_assume)).
Added @AnastasiaStulova for help with the OpenCL questions. I would love to avoid adding omp_assume if possible.
It should be noted that, without the omp_assume spelling, it would be impossible to use this attribute in C without running into -pedantic warnings; we could consider supporting [[omp::assume]] in C23, though.
My guess is that the OpenMP folks haven't gotten around to rebasing on top of C23 yet and that's really the only thing holding back supporting [[]] spellings in C with OpenMP. @alexey-bataev, should we enable OpenMP attribute spellings whenever double square bracket attributes are enabled?
From what I can tell, no-one except libclc is actually using this attribute? At least on github, the only matches I’ve found for attribute((assume("omp are in LLVM and various forks thereof. Given that it’s not particularly widely used, I don’t think removal without deprecation would be that big of a deal in this case, though if you think that that’s a non-option, then I’d instead suggest deprecating it and removing it in a later version of Clang.
This matches my searching around on https://sourcegraph.com/search?q=context:global+__attribute__%28%28assume%28%22omp+-file:.*test.*+-file:.*llvm.*+-file:.*%5C.rst&patternType=keyword&sm=0 so I think removal without deprecation may be reasonable unless @alexey-bataev knows of something we're missing.
Thanks, I didn’t know who to ping about that, and yeah, same. |
Idon'think there are any. @jdoerfert thoughts? |
|
OpenCL is "C" based, not C++. However, C23 DID add namespacing to attributes, so if OpenCL can use C23, we should be able to make the spelling |
| @@ -1,14 +0,0 @@ | |||
| // RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s | |||
There was a problem hiding this comment.
Same here, can we just rename this to attr-omp-assume.c and change the name in the test?
| @@ -1,58 +0,0 @@ | |||
| // RUN: %clang_cc1 -emit-llvm -triple i386-linux-gnu %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
would be great if we could just rename/update this test rather than deleting it?
There was a problem hiding this comment.
The main reason I outright deleted it is because I’m hoping we won’t be needing the __attribute__((omp_assume)) spelling, in which case these test cases become pointless since we’re already testing all of this w/ the [[omp::assume]] spelling; that said, if it turns out that we do need omp_assume, then I’ll add these back with that spelling instead.
I could add that in a separate pr; that means that one will have to be merged before this one because we have to replace that one use of |
|
I'll make a PR for clspv then |
Yep, understood. I'll do my best to do quick review cycles on the other patch. |
Alright, I’ll do that then; as I said though, I’m a bit busy, so I’ll probably only get to this sometime next week unfortunately... |
Note @rjodinchr has offered to do the PR for that one, so hopefully he and I will be able to cycle on it and get it committed before you could update this anyway. |
Instead add a proper attribute in clang, and add convert it to function metadata to keep the information in the IR. The goal is to remove the dependency on __attribute__((assume)) that should have not be there in the first place. Ref llvm#84934
|
Here is the PR ready for review: #92126 |
Instead add a proper attribute in clang, and add convert it to function metadata to keep the information in the IR. The goal is to remove the dependency on __attribute__((assume)) that should have not be there in the first place. Ref llvm#84934
Instead add a proper attribute in clang, and add convert it to function metadata to keep the information in the IR. The goal is to remove the dependency on __attribute__((assume)) that should have not be there in the first place. Ref llvm#84934
Instead add a proper attribute in clang, and add convert it to function metadata to keep the information in the IR. The goal is to remove the dependency on __attribute__((assume)) that should have not be there in the first place. Ref llvm#84934
Instead add a proper attribute in clang, and add convert it to function metadata to keep the information in the IR. The goal is to remove the dependency on __attribute__((assume)) that should have not be there in the first place. Ref #84934
|
Ok, I’ve removed the |
|
One last thing: the GNU |
|
Ok, the only CI failure is apparently |
@erichkeane @AaronBallman Just wanted to double-check whether this is fine because I committed this change after this pr was approved. |
Yep, that makes sense to me. Still LGTM. |
@AaronBallman, @erichkeane This is a followup to #81014 and #84582. For anyone not familiar with what this is about: Clang provides
__attribute__((assume))and[[clang::assume]]as nonstandard spellings for the[[omp::assume]]attribute; this results in a potentially very confusing name clash with C++23’s[[assume]]attribute. This pr is about deciding what to do about this.Just to see how much would be affected by this, I’ve gone ahead and removed every last mention of
__attribute__((assume))and replaced it with[[omp::assume]]. Here are a few things I have observed / changes I’ve implemented:[[omp::assume]]can no longer be spelt__attribute__((assume))or[[clang::assume]].__attribute__((assume))and[[clang::assume]]are now alternative spellings for C++23’s[[assume]]; this shouldn’t cause any problems due to differences in appertainment: the OMP attribute only appertains to functions, and C++23’s only to empty statements, so any code using__attribute__((assume))in that position with Clang would not have compiled anyway. This should also help improve GCC compatibility, as GCC supports__attribute__((assume))with the C++23 meaning.The OpenMP specification only supports
assumeas an attribute in C++11 and later. However, libclc appears to be using__attribute__((assume))internally, specifically, in one header that defines a macro that is then used throughout the codebase. I’m not familiar with libclc or OpenCL, so I’ve addedomp_assumeas an alternative spelling for the OpenMP attribute and replaced the attribute in the header in question with__attribute__((__omp_assume__)).If it turns out that using
[[]]-style attributes in libclc is fine after all, I would suggest using that instead and not providing theomp_assumespelling in the first place. It should be noted that, without theomp_assumespelling, it would be impossible to use this attribute in C without running into-pedanticwarnings; we could consider supporting[[omp::assume]]in C23, though.From what I can tell, no-one except libclc is actually using this attribute? At least on github, the only matches I’ve found for
__attribute__((assume("ompare in LLVM and various forks thereof. Given that it’s not particularly widely used, I don’t think removal without deprecation would be that big of a deal in this case, though if you think that that’s a non-option, then I’d instead suggest deprecating it and removing it in a later version of Clang.TODO:
[[omp::assume]]in libclc.[[omp::assume]]in C (or an alternative spelling thereof, in which case we should also update diagnostics that mention[[omp::assume]]to display that spelling instead if we’re compiling C, if possible), add some tests for that.__attribute__((assume))and C++23’s[[assume]].