[flang][Semantics][OpenMP] Check type of reduction variables #94596
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes llvm#92440 I had to delete part of reduction09.f90 because I don't think that should have ever worked.
tblah
requested review from
DavidTruby,
kiranchandramohan,
pawosm-arm,
klausler and
NimishMishra
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: Tom Eccles (tblah) ChangesFixes #92440 I had to delete part of reduction09.f90 because I don't think that should have ever worked. Full diff: https://github.com/llvm/llvm-project/pull/94596.diff 4 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 54ce45157537c..eba6da80d6264 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2378,6 +2378,89 @@ bool OmpStructureChecker::CheckIntrinsicOperator(
return false;
}
+static bool isReductionAllowedForType(
+ const parser::OmpClause::Reduction &x, const DeclTypeSpec *type) {
+ assert(type && "no type for reduction symbol");
+ const auto &definedOp{std::get<parser::OmpReductionOperator>(x.v.t)};
+ // TODO: user defined reduction operators. Just allow everything for now.
+ bool ok{true};
+
+ auto isLogical = [](const DeclTypeSpec *type) -> bool {
+ return type->category() == DeclTypeSpec::Logical;
+ };
+ auto isCharacter = [](const DeclTypeSpec *type) -> bool {
+ return type->category() == DeclTypeSpec::Character;
+ };
+
+ common::visit(
+ common::visitors{
+ [&](const parser::DefinedOperator &dOpr) {
+ if (const auto *intrinsicOp{
+ std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+ &dOpr.u)}) {
+ // OMP5.2: The type [...] of a list item that appears in a
+ // reduction clause must be valid for the combiner expression
+ // See F2023: Table 10.2
+ // .LT., .LE., .GT., .GE. are handled as procedure designators
+ // below.
+ switch (*intrinsicOp) {
+ case parser::DefinedOperator::IntrinsicOperator::Multiply:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::Add:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::Subtract:
+ ok = type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real) ||
+ type->IsNumeric(TypeCategory::Complex);
+ break;
+
+ case parser::DefinedOperator::IntrinsicOperator::AND:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::OR:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::EQV:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::NEQV:
+ ok = isLogical(type);
+ break;
+
+ // Reduction identifier is not in OMP5.2 Table 5.2
+ default:
+ assert(false &&
+ "This should have been caught in CheckIntrinsicOperator");
+ ok = false;
+ break;
+ }
+ }
+ },
+ [&](const parser::ProcedureDesignator &procD) {
+ const parser::Name *name{std::get_if<parser::Name>(&procD.u)};
+ if (name && name->symbol) {
+ const SourceName &realName{name->symbol->GetUltimate().name()};
+ // OMP5.2: The type [...] of a list item that appears in a
+ // reduction clause must be valid for the combiner expression
+ if (realName == "iand" || realName == "ior" ||
+ realName == "ieor") {
+ // IAND: arguments must be integers: F2023 16.9.100
+ // IEOR: arguments must be integers: F2023 16.9.106
+ // IOR: arguments must be integers: F2023 16.9.111
+ ok = type->IsNumeric(TypeCategory::Integer);
+ } else if (realName == "max" || realName == "min") {
+ // MAX: arguments must be integer, real, or character:
+ // F2023 16.9.135
+ // MIN: arguments must be integer, real, or character:
+ // F2023 16.9.141
+ ok = type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real) || isCharacter(type);
+ }
+ }
+ },
+ },
+ definedOp.u);
+
+ return ok;
+}
+
void OmpStructureChecker::CheckReductionTypeList(
const parser::OmpClause::Reduction &x) {
const auto &ompObjectList{std::get<parser::OmpObjectList>(x.v.t)};
@@ -2397,6 +2480,10 @@ void OmpStructureChecker::CheckReductionTypeList(
context_.Say(source,
"A procedure pointer '%s' must not appear in a REDUCTION clause."_err_en_US,
symbol->name());
+ } else if (!isReductionAllowedForType(x, symbol->GetType())) {
+ context_.Say(source,
+ "The type of '%s' is incompatible with the reduction operator."_err_en_US,
+ symbol->name());
}
}
}
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90 b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
index 8bded2fdb7469..ac6fe414d3044 100644
--- a/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
+++ b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
@@ -1,7 +1,8 @@
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! CHECK: not yet implemented: Reduction of some types is not supported
+! THere's no definition of '+' for type(t)
+! CHECK: The type of 'mt' is incompatible with the reduction operator.
subroutine reduction_allocatable
type t
integer :: x
diff --git a/flang/test/Semantics/OpenMP/reduction09.f90 b/flang/test/Semantics/OpenMP/reduction09.f90
index 095b49ba0c400..dbc8d1b060e65 100644
--- a/flang/test/Semantics/OpenMP/reduction09.f90
+++ b/flang/test/Semantics/OpenMP/reduction09.f90
@@ -73,14 +73,4 @@ program omp_reduction
k = k+1
end do
!$omp end do
-
-
- !$omp do reduction(.and.:k) reduction(.or.:j) reduction(.eqv.:l)
- !DEF: /omp_reduction/OtherConstruct8/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
- do i=1,10
- !DEF: /omp_reduction/OtherConstruct8/k (OmpReduction) HostAssoc INTEGER(4)
- k = k+1
- end do
- !$omp end do
-
end program omp_reduction
diff --git a/flang/test/Semantics/OpenMP/reduction14.f90 b/flang/test/Semantics/OpenMP/reduction14.f90
new file mode 100644
index 0000000000000..7d3f8d468f765
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction14.f90
@@ -0,0 +1,97 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.6 Reduction Clause
+program omp_reduction
+ integer :: i
+ real :: r
+ character :: c
+ complex :: z
+ logical :: l
+
+ ! * is allowed for integer, real, and complex
+ ! but not for logical or character
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(*:i,r,c,z,l)
+ !$omp end parallel
+
+ ! + is allowed for integer, real, and complex
+ ! but not for logical or character
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(+:i,r,c,z,l)
+ !$omp end parallel
+
+ ! - is deprecated for all types
+ ! ERROR: The minus reduction operator is deprecated since OpenMP 5.2 and is not supported in the REDUCTION clause.
+ !$omp parallel reduction(-:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .and. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.and.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .or. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.or.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .eqv. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.eqv.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .neqv. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.neqv.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! iand only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(iand:i,r,c,z,l)
+ !$omp end parallel
+
+ ! ior only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(ior:i,r,c,z,l)
+ !$omp end parallel
+
+ ! ieor only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(ieor:i,r,c,z,l)
+ !$omp end parallel
+
+ ! max arguments may be integer, real, or character:
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(max:i,r,c,z,l)
+ !$omp end parallel
+
+ ! min arguments may be integer, real, or character:
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(min:i,r,c,z,l)
+ !$omp end parallel
+end program omp_reduction
|
|
@llvm/pr-subscribers-flang-openmp Author: Tom Eccles (tblah) ChangesFixes #92440 I had to delete part of reduction09.f90 because I don't think that should have ever worked. Full diff: https://github.com/llvm/llvm-project/pull/94596.diff 4 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 54ce45157537c..eba6da80d6264 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2378,6 +2378,89 @@ bool OmpStructureChecker::CheckIntrinsicOperator(
return false;
}
+static bool isReductionAllowedForType(
+ const parser::OmpClause::Reduction &x, const DeclTypeSpec *type) {
+ assert(type && "no type for reduction symbol");
+ const auto &definedOp{std::get<parser::OmpReductionOperator>(x.v.t)};
+ // TODO: user defined reduction operators. Just allow everything for now.
+ bool ok{true};
+
+ auto isLogical = [](const DeclTypeSpec *type) -> bool {
+ return type->category() == DeclTypeSpec::Logical;
+ };
+ auto isCharacter = [](const DeclTypeSpec *type) -> bool {
+ return type->category() == DeclTypeSpec::Character;
+ };
+
+ common::visit(
+ common::visitors{
+ [&](const parser::DefinedOperator &dOpr) {
+ if (const auto *intrinsicOp{
+ std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+ &dOpr.u)}) {
+ // OMP5.2: The type [...] of a list item that appears in a
+ // reduction clause must be valid for the combiner expression
+ // See F2023: Table 10.2
+ // .LT., .LE., .GT., .GE. are handled as procedure designators
+ // below.
+ switch (*intrinsicOp) {
+ case parser::DefinedOperator::IntrinsicOperator::Multiply:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::Add:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::Subtract:
+ ok = type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real) ||
+ type->IsNumeric(TypeCategory::Complex);
+ break;
+
+ case parser::DefinedOperator::IntrinsicOperator::AND:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::OR:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::EQV:
+ [[fallthrough]];
+ case parser::DefinedOperator::IntrinsicOperator::NEQV:
+ ok = isLogical(type);
+ break;
+
+ // Reduction identifier is not in OMP5.2 Table 5.2
+ default:
+ assert(false &&
+ "This should have been caught in CheckIntrinsicOperator");
+ ok = false;
+ break;
+ }
+ }
+ },
+ [&](const parser::ProcedureDesignator &procD) {
+ const parser::Name *name{std::get_if<parser::Name>(&procD.u)};
+ if (name && name->symbol) {
+ const SourceName &realName{name->symbol->GetUltimate().name()};
+ // OMP5.2: The type [...] of a list item that appears in a
+ // reduction clause must be valid for the combiner expression
+ if (realName == "iand" || realName == "ior" ||
+ realName == "ieor") {
+ // IAND: arguments must be integers: F2023 16.9.100
+ // IEOR: arguments must be integers: F2023 16.9.106
+ // IOR: arguments must be integers: F2023 16.9.111
+ ok = type->IsNumeric(TypeCategory::Integer);
+ } else if (realName == "max" || realName == "min") {
+ // MAX: arguments must be integer, real, or character:
+ // F2023 16.9.135
+ // MIN: arguments must be integer, real, or character:
+ // F2023 16.9.141
+ ok = type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real) || isCharacter(type);
+ }
+ }
+ },
+ },
+ definedOp.u);
+
+ return ok;
+}
+
void OmpStructureChecker::CheckReductionTypeList(
const parser::OmpClause::Reduction &x) {
const auto &ompObjectList{std::get<parser::OmpObjectList>(x.v.t)};
@@ -2397,6 +2480,10 @@ void OmpStructureChecker::CheckReductionTypeList(
context_.Say(source,
"A procedure pointer '%s' must not appear in a REDUCTION clause."_err_en_US,
symbol->name());
+ } else if (!isReductionAllowedForType(x, symbol->GetType())) {
+ context_.Say(source,
+ "The type of '%s' is incompatible with the reduction operator."_err_en_US,
+ symbol->name());
}
}
}
diff --git a/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90 b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
index 8bded2fdb7469..ac6fe414d3044 100644
--- a/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
+++ b/flang/test/Lower/OpenMP/Todo/reduction-derived-type-field.f90
@@ -1,7 +1,8 @@
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
-! CHECK: not yet implemented: Reduction of some types is not supported
+! THere's no definition of '+' for type(t)
+! CHECK: The type of 'mt' is incompatible with the reduction operator.
subroutine reduction_allocatable
type t
integer :: x
diff --git a/flang/test/Semantics/OpenMP/reduction09.f90 b/flang/test/Semantics/OpenMP/reduction09.f90
index 095b49ba0c400..dbc8d1b060e65 100644
--- a/flang/test/Semantics/OpenMP/reduction09.f90
+++ b/flang/test/Semantics/OpenMP/reduction09.f90
@@ -73,14 +73,4 @@ program omp_reduction
k = k+1
end do
!$omp end do
-
-
- !$omp do reduction(.and.:k) reduction(.or.:j) reduction(.eqv.:l)
- !DEF: /omp_reduction/OtherConstruct8/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
- do i=1,10
- !DEF: /omp_reduction/OtherConstruct8/k (OmpReduction) HostAssoc INTEGER(4)
- k = k+1
- end do
- !$omp end do
-
end program omp_reduction
diff --git a/flang/test/Semantics/OpenMP/reduction14.f90 b/flang/test/Semantics/OpenMP/reduction14.f90
new file mode 100644
index 0000000000000..7d3f8d468f765
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction14.f90
@@ -0,0 +1,97 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.6 Reduction Clause
+program omp_reduction
+ integer :: i
+ real :: r
+ character :: c
+ complex :: z
+ logical :: l
+
+ ! * is allowed for integer, real, and complex
+ ! but not for logical or character
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(*:i,r,c,z,l)
+ !$omp end parallel
+
+ ! + is allowed for integer, real, and complex
+ ! but not for logical or character
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(+:i,r,c,z,l)
+ !$omp end parallel
+
+ ! - is deprecated for all types
+ ! ERROR: The minus reduction operator is deprecated since OpenMP 5.2 and is not supported in the REDUCTION clause.
+ !$omp parallel reduction(-:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .and. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.and.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .or. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.or.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .eqv. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.eqv.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! .neqv. is only supported for logical operations
+ ! ERROR: The type of 'i' is incompatible with the reduction operator.
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ !$omp parallel reduction(.neqv.:i,r,c,z,l)
+ !$omp end parallel
+
+ ! iand only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(iand:i,r,c,z,l)
+ !$omp end parallel
+
+ ! ior only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(ior:i,r,c,z,l)
+ !$omp end parallel
+
+ ! ieor only supports integers
+ ! ERROR: The type of 'r' is incompatible with the reduction operator.
+ ! ERROR: The type of 'c' is incompatible with the reduction operator.
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(ieor:i,r,c,z,l)
+ !$omp end parallel
+
+ ! max arguments may be integer, real, or character:
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(max:i,r,c,z,l)
+ !$omp end parallel
+
+ ! min arguments may be integer, real, or character:
+ ! ERROR: The type of 'z' is incompatible with the reduction operator.
+ ! ERROR: The type of 'l' is incompatible with the reduction operator.
+ !$omp parallel reduction(min:i,r,c,z,l)
+ !$omp end parallel
+end program omp_reduction
|
tschuett
reviewed
Jun 6, 2024
klausler
reviewed
Jun 6, 2024
| return type->category() == DeclTypeSpec::Character; | ||
| }; | ||
|
|
||
| common::visit( |
There was a problem hiding this comment.
Perhaps you could abstract CheckReduce() in lib/Semantics/check-cuda.cpp and also use it here.
There was a problem hiding this comment.
Thanks for the suggestion. The two functions are similar but not identical:
- No complex numbers in CUDA
- No subtract in CUDA (although perhaps we don't need to handle that here for OpenMP because it is currently a compiler error to use subtract anyway).
- OpenMP describes iand,ior,ieor,min, and max differently. The OpenMP 5.2 standard (5.5.1) does distinguish between operators and intrinsic procedure names, however I don't think there are any substantive differences between how these are treated.
@kiranchandramohan what do you think? Would it be a good fit to change the representation to use parser::ReductionOperator instead of parser::IntrinsicOperator for these built in reduction operations, so that we can then refactor to share with the CUDA code? I think as a side effect this could let us use the existing array reduction machinery for min and max without additional work (other than tests).
There was a problem hiding this comment.
For builtin reductions it might be OK. At the parser level, it might not work for renames (#68654). In OpenMP there is also user-defined declare reductions and reductions/user-defined operators.
There was a problem hiding this comment.
Yeah we would have to keep the ProcedureDesignator (or similar) around, I just mean not using it for the built in reduction intrinsic procedures.
There was a problem hiding this comment.
Would that mean two different paths for a built-in reduction intrinsic and its renamed version?
There was a problem hiding this comment.
I guess it would yes. Maybe that makes this a bad idea.
| } | ||
| } | ||
| }, | ||
| [&](const parser::ProcedureDesignator &procD) { |
There was a problem hiding this comment.
I'm surprised that the parse tree for OMP allows an arbitrary procedure designator here.
There was a problem hiding this comment.
Besides the allowed intrinsic procedures (max, min, iand, ior, ieor), for OpenMP the reduction identifier can be a base language identifier, or a user-defined operator. The latter appears as a ProcedureDesignator. We also had instances of renaming of intrinsics (#68654).
There was a problem hiding this comment.
Sure, but can it be a procedure component or a procedure binding?
There was a problem hiding this comment.
I think they cannot be a procedure component or binding. Are you suggesting changing the reduction-identifier from a ProcedureDesignator to a Name for procedure-name?
There was a problem hiding this comment.
So long as a procedure component or procedure binding is caught as an error somewhere in semantics, it doesn't need to be a parsing issue. A parser::Name wouldn't be enough, I don't think, because operators are also acceptable. If procedure components or procedure bindings are not caught in semantics, I suggest defining a new parse tree node and corresponding parser that are limited to what is allowed to appear only.
|
@klausler is there anything more you would like to see changed here? |
klausler
reviewed
Jun 12, 2024
klausler
approved these changes
Jun 12, 2024
EthanLuisMcDonough
pushed a commit
to EthanLuisMcDonough/llvm-project
that referenced
this pull request
Aug 13, 2024
) Fixes llvm#92440 I had to delete part of reduction09.f90 because I don't think that should have ever worked.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #92440
I had to delete part of reduction09.f90 because I don't think that should have ever worked.