[flang][OpenMP] Reuse semantic check for "constantness" of alignment #163624
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
Use ScalarIntConstantExpr in the parse tree instead of ScalarIntExpr. This will still parse a general expression, but the semantic checker for expressions will automatically perfom a test for whether the value is constant or not. Use that instead of manual checks, it will make diagnostics more uniform. There is no functional change other than that.
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-parser Author: Krzysztof Parzyszek (kparzysz) ChangesUse ScalarIntConstantExpr in the parse tree instead of ScalarIntExpr. This will still parse a general expression, but the semantic checker for expressions will automatically perfom a test for whether the value is constant or not. Use that instead of manual checks, it will make diagnostics more uniform. There is no functional change other than that. Full diff: https://github.com/llvm/llvm-project/pull/163624.diff 5 Files Affected:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d919b777d7487..0f1bab143dfc2 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4189,7 +4189,7 @@ struct OmpAffinityClause {
// Ref: 5.2: [174]
struct OmpAlignClause {
- WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntExpr);
+ WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntConstantExpr);
};
// Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 9507021057476..885f27742bcd4 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1085,7 +1085,7 @@ TYPE_PARSER(construct<OmpBindClause>(
"TEAMS" >> pure(OmpBindClause::Binding::Teams) ||
"THREAD" >> pure(OmpBindClause::Binding::Thread)))
-TYPE_PARSER(construct<OmpAlignClause>(scalarIntExpr))
+TYPE_PARSER(construct<OmpAlignClause>(scalarIntConstantExpr))
TYPE_PARSER(construct<OmpAtClause>(
"EXECUTION" >> pure(OmpAtClause::ActionTime::Execution) ||
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index d65a89e768466..5db54d17ebb8c 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1540,9 +1540,8 @@ void OmpStructureChecker::Leave(const parser::OpenMPRequiresConstruct &) {
void OmpStructureChecker::CheckAlignValue(const parser::OmpClause &clause) {
if (auto *align{std::get_if<parser::OmpClause::Align>(&clause.u)}) {
- if (const auto &v{GetIntValue(align->v)}; !v || *v <= 0) {
- context_.Say(clause.source,
- "The alignment value should be a constant positive integer"_err_en_US);
+ if (const auto &v{GetIntValue(align->v)}; v && *v <= 0) {
+ context_.Say(clause.source, "The alignment should be positive"_err_en_US);
}
}
}
diff --git a/flang/test/Parser/OpenMP/allocate-align-tree.f90 b/flang/test/Parser/OpenMP/allocate-align-tree.f90
index 8cb009dfe46c8..0d247cd1ed945 100644
--- a/flang/test/Parser/OpenMP/allocate-align-tree.f90
+++ b/flang/test/Parser/OpenMP/allocate-align-tree.f90
@@ -26,14 +26,14 @@ end program allocate_align_tree
!CHECK: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPExecutableAllocate
!CHECK-NEXT: | | | Verbatim
!CHECK-NEXT: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'xarray'
-!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '32_4'
+!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Constant -> Expr = '32_4'
!CHECK-NEXT: | | | | LiteralConstant -> IntLiteralConstant = '32'
!CHECK-NEXT: | | | OmpClause -> Allocator -> Scalar -> Integer -> Expr = '2_8'
!CHECK-NEXT: | | | | Designator -> DataRef -> Name = 'omp_large_cap_mem_alloc'
!CHECK-NEXT: | | | OpenMPDeclarativeAllocate
!CHECK-NEXT: | | | | Verbatim
!CHECK-NEXT: | | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j'
-!CHECK-NEXT: | | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '16_4'
+!CHECK-NEXT: | | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Constant -> Expr = '16_4'
!CHECK-NEXT: | | | | | LiteralConstant -> IntLiteralConstant = '16'
!CHECK-NEXT: | | | AllocateStmt
diff --git a/flang/test/Semantics/OpenMP/allocate-align01.f90 b/flang/test/Semantics/OpenMP/allocate-align01.f90
index 4967330e37b48..88bcd6d2f1008 100644
--- a/flang/test/Semantics/OpenMP/allocate-align01.f90
+++ b/flang/test/Semantics/OpenMP/allocate-align01.f90
@@ -11,10 +11,10 @@ program allocate_align_tree
integer :: z, t, xx
t = 2
z = 3
- !ERROR: The alignment value should be a constant positive integer
+ !ERROR: Must be a constant value
!$omp allocate(j) align(xx)
!WARNING: The executable form of the OpenMP ALLOCATE directive has been deprecated, please use ALLOCATORS instead [-Wopen-mp-usage]
- !ERROR: The alignment value should be a constant positive integer
+ !ERROR: The alignment should be positive
!$omp allocate(xarray) align(-32) allocator(omp_large_cap_mem_alloc)
allocate(j(z), xarray(t))
end program allocate_align_tree
|
Member
|
@llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesUse ScalarIntConstantExpr in the parse tree instead of ScalarIntExpr. This will still parse a general expression, but the semantic checker for expressions will automatically perfom a test for whether the value is constant or not. Use that instead of manual checks, it will make diagnostics more uniform. There is no functional change other than that. Full diff: https://github.com/llvm/llvm-project/pull/163624.diff 5 Files Affected:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d919b777d7487..0f1bab143dfc2 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4189,7 +4189,7 @@ struct OmpAffinityClause {
// Ref: 5.2: [174]
struct OmpAlignClause {
- WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntExpr);
+ WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntConstantExpr);
};
// Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 9507021057476..885f27742bcd4 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1085,7 +1085,7 @@ TYPE_PARSER(construct<OmpBindClause>(
"TEAMS" >> pure(OmpBindClause::Binding::Teams) ||
"THREAD" >> pure(OmpBindClause::Binding::Thread)))
-TYPE_PARSER(construct<OmpAlignClause>(scalarIntExpr))
+TYPE_PARSER(construct<OmpAlignClause>(scalarIntConstantExpr))
TYPE_PARSER(construct<OmpAtClause>(
"EXECUTION" >> pure(OmpAtClause::ActionTime::Execution) ||
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index d65a89e768466..5db54d17ebb8c 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1540,9 +1540,8 @@ void OmpStructureChecker::Leave(const parser::OpenMPRequiresConstruct &) {
void OmpStructureChecker::CheckAlignValue(const parser::OmpClause &clause) {
if (auto *align{std::get_if<parser::OmpClause::Align>(&clause.u)}) {
- if (const auto &v{GetIntValue(align->v)}; !v || *v <= 0) {
- context_.Say(clause.source,
- "The alignment value should be a constant positive integer"_err_en_US);
+ if (const auto &v{GetIntValue(align->v)}; v && *v <= 0) {
+ context_.Say(clause.source, "The alignment should be positive"_err_en_US);
}
}
}
diff --git a/flang/test/Parser/OpenMP/allocate-align-tree.f90 b/flang/test/Parser/OpenMP/allocate-align-tree.f90
index 8cb009dfe46c8..0d247cd1ed945 100644
--- a/flang/test/Parser/OpenMP/allocate-align-tree.f90
+++ b/flang/test/Parser/OpenMP/allocate-align-tree.f90
@@ -26,14 +26,14 @@ end program allocate_align_tree
!CHECK: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPExecutableAllocate
!CHECK-NEXT: | | | Verbatim
!CHECK-NEXT: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'xarray'
-!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '32_4'
+!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Constant -> Expr = '32_4'
!CHECK-NEXT: | | | | LiteralConstant -> IntLiteralConstant = '32'
!CHECK-NEXT: | | | OmpClause -> Allocator -> Scalar -> Integer -> Expr = '2_8'
!CHECK-NEXT: | | | | Designator -> DataRef -> Name = 'omp_large_cap_mem_alloc'
!CHECK-NEXT: | | | OpenMPDeclarativeAllocate
!CHECK-NEXT: | | | | Verbatim
!CHECK-NEXT: | | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j'
-!CHECK-NEXT: | | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '16_4'
+!CHECK-NEXT: | | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Constant -> Expr = '16_4'
!CHECK-NEXT: | | | | | LiteralConstant -> IntLiteralConstant = '16'
!CHECK-NEXT: | | | AllocateStmt
diff --git a/flang/test/Semantics/OpenMP/allocate-align01.f90 b/flang/test/Semantics/OpenMP/allocate-align01.f90
index 4967330e37b48..88bcd6d2f1008 100644
--- a/flang/test/Semantics/OpenMP/allocate-align01.f90
+++ b/flang/test/Semantics/OpenMP/allocate-align01.f90
@@ -11,10 +11,10 @@ program allocate_align_tree
integer :: z, t, xx
t = 2
z = 3
- !ERROR: The alignment value should be a constant positive integer
+ !ERROR: Must be a constant value
!$omp allocate(j) align(xx)
!WARNING: The executable form of the OpenMP ALLOCATE directive has been deprecated, please use ALLOCATORS instead [-Wopen-mp-usage]
- !ERROR: The alignment value should be a constant positive integer
+ !ERROR: The alignment should be positive
!$omp allocate(xarray) align(-32) allocator(omp_large_cap_mem_alloc)
allocate(j(z), xarray(t))
end program allocate_align_tree
|
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.
Use ScalarIntConstantExpr in the parse tree instead of ScalarIntExpr. This will still parse a general expression, but the semantic checker for expressions will automatically perfom a test for whether the value is constant or not.
Use that instead of manual checks, it will make diagnostics more uniform. There is no functional change other than that.