[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155635/new/

https://reviews.llvm.org/D155635

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Fazlay Rabbi via Phabricator via cfe-commits]

mdfazlay added a comment.

In D155635#4512243 , @ABataev wrote:

> In D155635#4512188 , @mdfazlay 
> wrote:
>
>> In D155635#4512183 , @ABataev 
>> wrote:
>>
>>> Tests?
>>
>> I think I need to use `-fopenmp-version=60` to test this revision. As OMP 
>> 6.0 is not official yet, am I allowed to use `-fopenmp-version=60` in my LIT 
>> test?
>
> Yes

I have added a few LIT tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155635/new/

https://reviews.llvm.org/D155635

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D155635#4512188 , @mdfazlay wrote:

> In D155635#4512183 , @ABataev wrote:
>
>> Tests?
>
> I think I need to use `-fopenmp-version=60` to test this revision. As OMP 6.0 
> is not official yet, am I allowed to use `-fopenmp-version=60` in my LIT test?

Yes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155635/new/

https://reviews.llvm.org/D155635

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Fazlay Rabbi via Phabricator via cfe-commits]

mdfazlay added a comment.

In D155635#4512183 , @ABataev wrote:

> Tests?

I think I need to use `-fopenmp-version=60` to test this revision. As OMP 6.0 
is not official yet, am I allowed to use `-fopenmp-version=60` in my LIT test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155635/new/

https://reviews.llvm.org/D155635

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Tests?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155635/new/

https://reviews.llvm.org/D155635

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155635: [OpenMP] [Reduction] Allow PLUS (+) operator on reduction clauses in OMP > 52

[Fazlay Rabbi via Phabricator via cfe-commits]

mdfazlay created this revision.
Herald added subscribers: sunshaoce, guansong, yaxunl.
Herald added a project: All.
mdfazlay requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

Currently, clang gives an incorrect reduction identifier error for the PLUS
operator for OpenMP version > 52. But, PLUS operator is allowed in OpenMP
version > 52. This revision fixes this issue and also modified the error
messages to show the correct expected operators in the message based on the 
OpenMP
version used (prior to OMP 6.0 and since OMP 6.0).

**Test Src:**

  void foo() {
   int a = 0 ;
#pragma omp parallel reduction(+:a)
   ;
 #pragma omp parallel reduction(-:a)
   ;
   }

**Before this revision:**

  $ clang -fopenmp -fopenmp-version=60 test.c -c
  test.c:3:34: error: incorrect reduction identifier, expected one of '+', '-', 
'*', '&', '|', '^', '&&', '||', 'min' or 'max' or declare reduction for type 
'int'
3 |   #pragma omp parallel reduction(+:a)
  |  ^
  test.c:5:34: error: incorrect reduction identifier, expected one of '+', '-', 
'*', '&', '|', '^', '&&', '||', 'min' or 'max' or declare reduction for type 
'int'
5 |   #pragma omp parallel reduction(-:a)
  |  ^
  2 errors generated.

**With this revision:**

  $  clang -fopenmp -fopenmp-version=60 test.c -c
  test.c:5:34: error: incorrect reduction identifier, expected one of '+', '*', 
'&', '|', '^', '&&', '||', 'min' or 'max' or declare reduction for type 'int'
  5 |   #pragma omp parallel reduction(-:a)
|  ^


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155635

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -19169,6 +19169,8 @@
   // operators: +, -, *, &, |, ^, && and ||
   switch (OOK) {
   case OO_Plus:
+BOK = BO_Add;
+break;
   case OO_Minus:
 // Minus(-) operator is not supported in TR11 (OpenMP 6.0). Setting BOK to
 // BO_Comma will automatically diagnose it for OpenMP > 52 as not allowed
@@ -19418,9 +19420,14 @@
 }
 if (BOK == BO_Comma && DeclareReductionRef.isUnset()) {
   // Not allowed reduction identifier is found.
-  S.Diag(ReductionId.getBeginLoc(),
- diag::err_omp_unknown_reduction_identifier)
-  << Type << ReductionIdRange;
+  if (S.LangOpts.OpenMP > 52)
+S.Diag(ReductionId.getBeginLoc(),
+   diag::err_omp_unknown_reduction_identifier_since_omp_6_0)
+<< Type << ReductionIdRange;
+  else
+S.Diag(ReductionId.getBeginLoc(),
+   diag::err_omp_unknown_reduction_identifier_prior_omp_6_0)
+<< Type << ReductionIdRange;
   continue;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10679,9 +10679,12 @@
 def warn_omp_loop_64_bit_var : Warning<
   "OpenMP loop iteration variable cannot have more than 64 bits size and will 
be narrowed">,
   InGroup;
-def err_omp_unknown_reduction_identifier : Error<
+def err_omp_unknown_reduction_identifier_prior_omp_6_0 : Error<
   "incorrect reduction identifier, expected one of '+', '-', '*', '&', '|', 
'^', "
   "'&&', '||', 'min' or 'max' or declare reduction for type %0">;
+def err_omp_unknown_reduction_identifier_since_omp_6_0 : Error<
+  "incorrect reduction identifier, expected one of '+', '*', '&', '|', '^', "
+  "'&&', '||', 'min' or 'max' or declare reduction for type %0">;
 def err_omp_not_resolved_reduction_identifier : Error<
   "unable to resolve declare reduction construct for type %0">;
 def err_omp_reduction_ref_type_arg : Error<


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -19169,6 +19169,8 @@
   // operators: +, -, *, &, |, ^, && and ||
   switch (OOK) {
   case OO_Plus:
+BOK = BO_Add;
+break;
   case OO_Minus:
 // Minus(-) operator is not supported in TR11 (OpenMP 6.0). Setting BOK to
 // BO_Comma will automatically diagnose it for OpenMP > 52 as not allowed
@@ -19418,9 +19420,14 @@
 }
 if (BOK == BO_Comma && DeclareReductionRef.isUnset()) {
   // Not allowed reduction identifier is found.
-  S.Diag(ReductionId.getBeginLoc(),
- diag::err_omp_unknown_reduction_identifier)
-  << Type << ReductionIdRange;
+  if (S.LangOpts.OpenMP > 52)
+S.Diag(ReductionId.getBeginLoc(),
+