[FLANG][OpenMP]Add support for ALIGN clause on OMP ALLOCATE #120791

Leporacanthicus · 2024-12-20T19:48:50Z

This is trivially additional support for the existing ALLOCATE directive, which allows an ALIGN clause.

The ALLOCATE directive is currently not implemented, so this is just addding the necessary parser parts to allow the compiler to not say "Huh? I don't get this" [or "Expected OpenMP construct"] when it encounters the ALIGN clause.

Some parser testing is updated and a new todo test, just in case the feature of align clause is not supported by the initial support for ALLOCATE.

This is trivially additional support for the existing ALLOCATE directive, which allows an ALIGN clause. The ALLOCATE directive is currently not implemented, so this is just addding the necessary parser parts to allow the compiler to not say "Huh? I don't get this" [or "Expected OpenMP construct"] when it encounters the ALIGN clause. Some parser testing is updated and a new todo test, just in case the feature of align clause is not supported by the initial support for ALLOCATE.

llvmbot · 2024-12-20T19:49:26Z

@llvm/pr-subscribers-flang-semantics
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Mats Petersson (Leporacanthicus)

Changes

This is trivially additional support for the existing ALLOCATE directive, which allows an ALIGN clause.

The ALLOCATE directive is currently not implemented, so this is just addding the necessary parser parts to allow the compiler to not say "Huh? I don't get this" [or "Expected OpenMP construct"] when it encounters the ALIGN clause.

Some parser testing is updated and a new todo test, just in case the feature of align clause is not supported by the initial support for ALLOCATE.

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

7 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+1)
(modified) flang/include/flang/Parser/parse-tree.h (+5)
(modified) flang/lib/Parser/openmp-parsers.cpp (+4)
(added) flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90 (+10)
(added) flang/test/Parser/OpenMP/allocate-align-tree.f90 (+30)
(modified) flang/test/Parser/OpenMP/allocate-unparse.f90 (+3-1)
(modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 940caaeea9c3b7..485e021ac39e90 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -486,6 +486,7 @@ class ParseTreeDumper {
   NODE(parser, OmpAffinityClause)
   NODE(OmpAffinityClause, Modifier)
   NODE(parser, OmpAlignment)
+  NODE(parser, OmpAlignClause)
   NODE(parser, OmpAlignedClause)
   NODE(OmpAlignedClause, Modifier)
   NODE(parser, OmpAtClause)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 1d97126d17dbc4..3c494fa5290a9c 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3752,6 +3752,11 @@ struct OmpAffinityClause {
   std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
+// Ref: 5.2: [174]
+struct OmpAlignClause {
+  WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntExpr);
+};
+
 // Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
 //
 // aligned-clause ->
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 67385c03f66c80..2c739378989dcd 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -567,6 +567,8 @@ TYPE_PARSER(construct<OmpBindClause>(
     "TEAMS" >> pure(OmpBindClause::Binding::Teams) ||
     "THREAD" >> pure(OmpBindClause::Binding::Thread)))
 
+TYPE_PARSER(construct<OmpAlignClause>(scalarIntExpr))
+
 TYPE_PARSER(construct<OmpAtClause>(
     "EXECUTION" >> pure(OmpAtClause::ActionTime::Execution) ||
     "COMPILATION" >> pure(OmpAtClause::ActionTime::Compilation)))
@@ -582,6 +584,8 @@ TYPE_PARSER(
     "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
     "AFFINITY" >> construct<OmpClause>(construct<OmpClause::Affinity>(
                       parenthesized(Parser<OmpAffinityClause>{}))) ||
+    "ALIGN" >> construct<OmpClause>(construct<OmpClause::Align>(
+                   parenthesized(Parser<OmpAlignClause>{}))) ||
     "ALIGNED" >> construct<OmpClause>(construct<OmpClause::Aligned>(
                      parenthesized(Parser<OmpAlignedClause>{}))) ||
     "ALLOCATE" >> construct<OmpClause>(construct<OmpClause::Allocate>(
diff --git a/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90 b/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90
new file mode 100644
index 00000000000000..d0ed0cbb4c831d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90
@@ -0,0 +1,10 @@
+! This test checks lowering of OpenMP allocate Directive with align clause.
+
+// RUN: not flang -fc1 -emit-fir -fopenmp %s 2>&1 | FileCheck %s
+
+program main
+  integer :: x
+
+  // CHECK: not yet implemented: OpenMPDeclarativeAllocate
+  !$omp allocate(x) align(32)
+end
diff --git a/flang/test/Parser/OpenMP/allocate-align-tree.f90 b/flang/test/Parser/OpenMP/allocate-align-tree.f90
new file mode 100644
index 00000000000000..7ff76ea30e2f2b
--- /dev/null
+++ b/flang/test/Parser/OpenMP/allocate-align-tree.f90
@@ -0,0 +1,30 @@
+! REQUIRES: openmp_runtime
+
+! RUN: %flang_fc1 %openmp_flags -fopenmp-version=51 -fdebug-dump-parse-tree %s | FileCheck %s
+! RUN: %flang_fc1 %openmp_flags -fdebug-unparse -fopenmp-version=51 %s | FileCheck %s --check-prefix="UNPARSE"
+! Ensures associated declarative OMP allocations are nested in their
+! corresponding executable allocate directive
+
+program allocate_tree
+    use omp_lib
+    integer, allocatable :: j
+!$omp allocate(j) align(16)    
+    allocate(j)
+end program allocate_tree
+
+!CHECK: | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
+!CHECK-NEXT: | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec ->
+!CHECK-NEXT: | | | AttrSpec -> Allocatable
+!CHECK-NEXT: | | | EntityDecl
+!CHECK-NEXT: | | | | Name = 'j'
+
+
+!CHECK: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPExecutableAllocate
+!CHECK-NEXT: | | | Verbatim
+!CHECK-NEXT: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j'
+!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '16_4'
+!CHECK-NEXT: | | | | LiteralConstant -> IntLiteralConstant = '16'
+!CHECK-NEXT: | | | AllocateStmt
+
+!UNPARSE: !$OMP ALLOCATE (j) ALIGN(16_4)
+!UNPARSE-NEXT: ALLOCATE(j)
diff --git a/flang/test/Parser/OpenMP/allocate-unparse.f90 b/flang/test/Parser/OpenMP/allocate-unparse.f90
index 81b3677ad954bd..94bc2adf35ea91 100644
--- a/flang/test/Parser/OpenMP/allocate-unparse.f90
+++ b/flang/test/Parser/OpenMP/allocate-unparse.f90
@@ -5,7 +5,7 @@ program allocate_unparse
 use omp_lib
 
 real, dimension (:,:), allocatable :: darray
-integer :: a, b, m, n, t, x, y, z
+integer :: a, b, j, m, n, t, x, y, z
 
 ! 2.11.3 declarative allocate
 
@@ -25,6 +25,7 @@ program allocate_unparse
 !$omp allocate(z) allocator(omp_default_mem_alloc)
 !$omp allocate(m) allocator(omp_default_mem_alloc)
 !$omp allocate(n)
+!$omp allocate(j) align(16)
     allocate ( darray(z, t) )
 
 end program allocate_unparse
@@ -41,4 +42,5 @@ end program allocate_unparse
 !CHECK:!$OMP ALLOCATE (z) ALLOCATOR(omp_default_mem_alloc)
 !CHECK:!$OMP ALLOCATE (m) ALLOCATOR(omp_default_mem_alloc)
 !CHECK:!$OMP ALLOCATE (n)
+!CHECK:!$OMP ALLOCATE (j) ALIGN(16)
 !CHECK:ALLOCATE(darray(z,t))
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e36eb77cefe7e3..a4c1964c3e88f5 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -49,6 +49,7 @@ def OMPC_Affinity : Clause<"affinity"> {
 }
 def OMPC_Align : Clause<"align"> {
   let clangClass = "OMPAlignClause";
+  let flangClass = "OmpAlignClause";
 }
 def OMPC_Aligned : Clause<"aligned"> {
   let clangClass = "OMPAlignedClause";

llvmbot · 2024-12-20T19:49:26Z

@llvm/pr-subscribers-flang-parser

Author: Mats Petersson (Leporacanthicus)

Changes

This is trivially additional support for the existing ALLOCATE directive, which allows an ALIGN clause.

The ALLOCATE directive is currently not implemented, so this is just addding the necessary parser parts to allow the compiler to not say "Huh? I don't get this" [or "Expected OpenMP construct"] when it encounters the ALIGN clause.

Some parser testing is updated and a new todo test, just in case the feature of align clause is not supported by the initial support for ALLOCATE.

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

7 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+1)
(modified) flang/include/flang/Parser/parse-tree.h (+5)
(modified) flang/lib/Parser/openmp-parsers.cpp (+4)
(added) flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90 (+10)
(added) flang/test/Parser/OpenMP/allocate-align-tree.f90 (+30)
(modified) flang/test/Parser/OpenMP/allocate-unparse.f90 (+3-1)
(modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 940caaeea9c3b7..485e021ac39e90 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -486,6 +486,7 @@ class ParseTreeDumper {
   NODE(parser, OmpAffinityClause)
   NODE(OmpAffinityClause, Modifier)
   NODE(parser, OmpAlignment)
+  NODE(parser, OmpAlignClause)
   NODE(parser, OmpAlignedClause)
   NODE(OmpAlignedClause, Modifier)
   NODE(parser, OmpAtClause)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 1d97126d17dbc4..3c494fa5290a9c 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3752,6 +3752,11 @@ struct OmpAffinityClause {
   std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
+// Ref: 5.2: [174]
+struct OmpAlignClause {
+  WRAPPER_CLASS_BOILERPLATE(OmpAlignClause, ScalarIntExpr);
+};
+
 // Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
 //
 // aligned-clause ->
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 67385c03f66c80..2c739378989dcd 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -567,6 +567,8 @@ TYPE_PARSER(construct<OmpBindClause>(
     "TEAMS" >> pure(OmpBindClause::Binding::Teams) ||
     "THREAD" >> pure(OmpBindClause::Binding::Thread)))
 
+TYPE_PARSER(construct<OmpAlignClause>(scalarIntExpr))
+
 TYPE_PARSER(construct<OmpAtClause>(
     "EXECUTION" >> pure(OmpAtClause::ActionTime::Execution) ||
     "COMPILATION" >> pure(OmpAtClause::ActionTime::Compilation)))
@@ -582,6 +584,8 @@ TYPE_PARSER(
     "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
     "AFFINITY" >> construct<OmpClause>(construct<OmpClause::Affinity>(
                       parenthesized(Parser<OmpAffinityClause>{}))) ||
+    "ALIGN" >> construct<OmpClause>(construct<OmpClause::Align>(
+                   parenthesized(Parser<OmpAlignClause>{}))) ||
     "ALIGNED" >> construct<OmpClause>(construct<OmpClause::Aligned>(
                      parenthesized(Parser<OmpAlignedClause>{}))) ||
     "ALLOCATE" >> construct<OmpClause>(construct<OmpClause::Allocate>(
diff --git a/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90 b/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90
new file mode 100644
index 00000000000000..d0ed0cbb4c831d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/omp-declarative-allocate-align.f90
@@ -0,0 +1,10 @@
+! This test checks lowering of OpenMP allocate Directive with align clause.
+
+// RUN: not flang -fc1 -emit-fir -fopenmp %s 2>&1 | FileCheck %s
+
+program main
+  integer :: x
+
+  // CHECK: not yet implemented: OpenMPDeclarativeAllocate
+  !$omp allocate(x) align(32)
+end
diff --git a/flang/test/Parser/OpenMP/allocate-align-tree.f90 b/flang/test/Parser/OpenMP/allocate-align-tree.f90
new file mode 100644
index 00000000000000..7ff76ea30e2f2b
--- /dev/null
+++ b/flang/test/Parser/OpenMP/allocate-align-tree.f90
@@ -0,0 +1,30 @@
+! REQUIRES: openmp_runtime
+
+! RUN: %flang_fc1 %openmp_flags -fopenmp-version=51 -fdebug-dump-parse-tree %s | FileCheck %s
+! RUN: %flang_fc1 %openmp_flags -fdebug-unparse -fopenmp-version=51 %s | FileCheck %s --check-prefix="UNPARSE"
+! Ensures associated declarative OMP allocations are nested in their
+! corresponding executable allocate directive
+
+program allocate_tree
+    use omp_lib
+    integer, allocatable :: j
+!$omp allocate(j) align(16)    
+    allocate(j)
+end program allocate_tree
+
+!CHECK: | | DeclarationConstruct -> SpecificationConstruct -> TypeDeclarationStmt
+!CHECK-NEXT: | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec ->
+!CHECK-NEXT: | | | AttrSpec -> Allocatable
+!CHECK-NEXT: | | | EntityDecl
+!CHECK-NEXT: | | | | Name = 'j'
+
+
+!CHECK: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPExecutableAllocate
+!CHECK-NEXT: | | | Verbatim
+!CHECK-NEXT: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j'
+!CHECK-NEXT: | | | OmpClauseList -> OmpClause -> Align -> OmpAlignClause -> Scalar -> Integer -> Expr = '16_4'
+!CHECK-NEXT: | | | | LiteralConstant -> IntLiteralConstant = '16'
+!CHECK-NEXT: | | | AllocateStmt
+
+!UNPARSE: !$OMP ALLOCATE (j) ALIGN(16_4)
+!UNPARSE-NEXT: ALLOCATE(j)
diff --git a/flang/test/Parser/OpenMP/allocate-unparse.f90 b/flang/test/Parser/OpenMP/allocate-unparse.f90
index 81b3677ad954bd..94bc2adf35ea91 100644
--- a/flang/test/Parser/OpenMP/allocate-unparse.f90
+++ b/flang/test/Parser/OpenMP/allocate-unparse.f90
@@ -5,7 +5,7 @@ program allocate_unparse
 use omp_lib
 
 real, dimension (:,:), allocatable :: darray
-integer :: a, b, m, n, t, x, y, z
+integer :: a, b, j, m, n, t, x, y, z
 
 ! 2.11.3 declarative allocate
 
@@ -25,6 +25,7 @@ program allocate_unparse
 !$omp allocate(z) allocator(omp_default_mem_alloc)
 !$omp allocate(m) allocator(omp_default_mem_alloc)
 !$omp allocate(n)
+!$omp allocate(j) align(16)
     allocate ( darray(z, t) )
 
 end program allocate_unparse
@@ -41,4 +42,5 @@ end program allocate_unparse
 !CHECK:!$OMP ALLOCATE (z) ALLOCATOR(omp_default_mem_alloc)
 !CHECK:!$OMP ALLOCATE (m) ALLOCATOR(omp_default_mem_alloc)
 !CHECK:!$OMP ALLOCATE (n)
+!CHECK:!$OMP ALLOCATE (j) ALIGN(16)
 !CHECK:ALLOCATE(darray(z,t))
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e36eb77cefe7e3..a4c1964c3e88f5 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -49,6 +49,7 @@ def OMPC_Affinity : Clause<"affinity"> {
 }
 def OMPC_Align : Clause<"align"> {
   let clangClass = "OMPAlignClause";
+  let flangClass = "OmpAlignClause";
 }
 def OMPC_Aligned : Clause<"aligned"> {
   let clangClass = "OMPAlignedClause";

tblah

LGTM thanks

kparzysz · 2025-01-02T20:30:19Z

Could you add a semantic check that the value is a compile-time positive constant, and some tests for it?

Leporacanthicus · 2025-01-03T18:43:32Z

Could you add a semantic check that the value is a compile-time positive constant, and some tests for it?

Done. Took a bit longer than I wanted, because I didn't realize my test-case was actually not going where it SHOULD go, so I added the executable variant, so it catches both types. And I had to add some more test-code to hit the declarative align side too. I guess whoever did the executable version never covered the align (or did I add that too?)

kparzysz

LGTM, thanks!

) This is trivially additional support for the existing ALLOCATE directive, which allows an ALIGN clause. The ALLOCATE directive is currently not implemented, so this is just addding the necessary parser parts to allow the compiler to not say "Huh? I don't get this" [or "Expected OpenMP construct"] when it encounters the ALIGN clause. Some parser testing is updated and a new todo test, just in case the feature of align clause is not supported by the initial support for ALLOCATE.

Leporacanthicus requested review from tblah, kiranchandramohan and kparzysz December 20, 2024 19:48

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:parser clang:openmp OpenMP related changes to Clang labels Dec 20, 2024

tblah approved these changes Jan 2, 2025

View reviewed changes

Add semantic check for align clause

8e75808

llvmbot added the flang:semantics label Jan 3, 2025

kparzysz approved these changes Jan 3, 2025

View reviewed changes

Leporacanthicus merged commit 4df366c into llvm:main Jan 6, 2025
9 checks passed

Leporacanthicus mentioned this pull request Jan 6, 2025

[Flang][OpenMP] Important issues to be fixed prior to removing experimental status of OpenMP #110008

Open

85 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[FLANG][OpenMP]Add support for ALIGN clause on OMP ALLOCATE #120791

[FLANG][OpenMP]Add support for ALIGN clause on OMP ALLOCATE #120791

Uh oh!

Leporacanthicus commented Dec 20, 2024

Uh oh!

llvmbot commented Dec 20, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

tblah left a comment

Uh oh!

kparzysz commented Jan 2, 2025

Uh oh!

Leporacanthicus commented Jan 3, 2025

Uh oh!

kparzysz left a comment

Uh oh!

Uh oh!

Uh oh!

[FLANG][OpenMP]Add support for ALIGN clause on OMP ALLOCATE #120791

[FLANG][OpenMP]Add support for ALIGN clause on OMP ALLOCATE #120791

Uh oh!

Conversation

Leporacanthicus commented Dec 20, 2024

Uh oh!

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

Uh oh!

Uh oh!

llvmbot commented Dec 20, 2024

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

kparzysz commented Jan 2, 2025

Uh oh!

Leporacanthicus commented Jan 3, 2025

Uh oh!

kparzysz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Dec 20, 2024 •

edited

Loading