[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG29aa749087be: [OpenMP][Flang][MLIR] Lowering of OpenMP 
requires directive from parse tree to… (authored by skatrak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
  flang/test/Lower/OpenMP/requires-common.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+  !$omp target
+  !$omp end target
+end program requires
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-common.f90
@@ -0,0 +1,19 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+block data init
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
@@ -0,0 +1,23 @@
+! This test checks the lowering of REQUIRES inside of an unnamed BLOCK DATA.
+! The symbol of the `symTab` scope of the `BlockDataUnit` PFT node is null in
+! this case, resulting in the inability to store the REQUIRES flags gathered in
+! it.
+
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+! XFAIL: *
+
+!CHECK: module attributes {
+!CHECK-SAME:omp.requires = #omp
+block data
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -78,9 +78,7 @@
 static void gatherFuncAndVarSyms(
 const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause,
-llvm::SmallVectorImpl>
-) {
+llvm::SmallVectorImpl ) {
   for (const Fortran::parser::OmpObject  : objList.v) {
 Fortran::common::visit(
 Fortran::common::visitors{
@@ -2453,6 +2451,71 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 556655.
skatrak added a comment.

Rebase and address review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
  flang/test/Lower/OpenMP/requires-common.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+  !$omp target
+  !$omp end target
+end program requires
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-common.f90
@@ -0,0 +1,19 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+block data init
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
@@ -0,0 +1,23 @@
+! This test checks the lowering of REQUIRES inside of an unnamed BLOCK DATA.
+! The symbol of the `symTab` scope of the `BlockDataUnit` PFT node is null in
+! this case, resulting in the inability to store the REQUIRES flags gathered in
+! it.
+
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+! XFAIL: *
+
+!CHECK: module attributes {
+!CHECK-SAME:omp.requires = #omp
+block data
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -78,9 +78,7 @@
 static void gatherFuncAndVarSyms(
 const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause,
-llvm::SmallVectorImpl>
-) {
+llvm::SmallVectorImpl ) {
   for (const Fortran::parser::OmpObject  : objList.v) {
 Fortran::common::visit(
 Fortran::common::visitors{
@@ -2474,6 +2472,71 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

In D147218#4645055 , @skatrak wrote:

> Thank you for the review. After I address your last comment my plan is to 
> land this and the other two accepted REQUIRES patches that depended on it.
>
> Is there a preferred approach as to how to go about it? I could rebase them 
> all and wait until the pre-merge builds finish without errors and then land 
> them in quick succession or I could go one by one to make sure post-merge 
> builds don't find any issues before landing the next, which will be over a 
> couple of days most likely.

Both are fine. I would recommend the former.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added a comment.

Thank you for the review. After I address your last comment my plan is to land 
this and the other two accepted REQUIRES patches that depended on it.

Is there a preferred approach as to how to go about it? I could rebase them all 
and wait until the pre-merge builds finish without errors and then land them in 
quick succession or I could go one by one to make sure post-merge builds don't 
find any issues before landing the next, which will be over a couple of days 
most likely.




Comment at: flang/lib/Lower/Bridge.cpp:2366-2367
 mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
+analyzeOpenMPDeclarativeConstruct(*this, getEval(), ompDecl,
+  ompDeviceCodeFound);
 genOpenMPDeclarativeConstruct(*this, getEval(), ompDecl);

kiranchandramohan wrote:
> Can this be rewritten this way.
> 
> And rename `analyzeOpenMPDeclarativeConstruct` to `isTargetDeclare` or 
> something like that?
My idea was to make it generic to allow the analysis of other declarative 
constructs in the future there as well, that's the reason for the function name 
and returning through an output argument. But I'll follow your suggestion, 
since we don't know for sure that we'll need this later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Kiran Chandramohan via Phabricator via cfe-commits]

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

LG.




Comment at: flang/lib/Lower/Bridge.cpp:2366-2367
 mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
+analyzeOpenMPDeclarativeConstruct(*this, getEval(), ompDecl,
+  ompDeviceCodeFound);
 genOpenMPDeclarativeConstruct(*this, getEval(), ompDecl);

Can this be rewritten this way.

And rename `analyzeOpenMPDeclarativeConstruct` to `isTargetDeclare` or 
something like that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 556543.
skatrak added a comment.

Remove leftover comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
  flang/test/Lower/OpenMP/requires-common.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+  !$omp target
+  !$omp end target
+end program requires
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-common.f90
@@ -0,0 +1,19 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+block data init
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
@@ -0,0 +1,23 @@
+! This test checks the lowering of REQUIRES inside of an unnamed BLOCK DATA.
+! The symbol of the `symTab` scope of the `BlockDataUnit` PFT node is null in
+! this case, resulting in the inability to store the REQUIRES flags gathered in
+! it.
+
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+! XFAIL: *
+
+!CHECK: module attributes {
+!CHECK-SAME:omp.requires = #omp
+block data
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -78,9 +78,7 @@
 static void gatherFuncAndVarSyms(
 const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause,
-llvm::SmallVectorImpl>
-) {
+llvm::SmallVectorImpl ) {
   for (const Fortran::parser::OmpObject  : objList.v) {
 Fortran::common::visit(
 Fortran::common::visitors{
@@ -2474,6 +2472,71 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added a comment.

Thank you Kiran for having a look at this!

In D147218#4643257 , 
@kiranchandramohan wrote:

> Could you add to the summary that the `atomic` related handling is done 
> elsewhere.

Done.

> Could you expand the tests to cover the various `if` conditions that are used 
> in the code?

I created a test to gather the flags from the symbol for a `FunctionLikeUnit` 
and for a `BlockDataUnit`, and checked that these flags are only added for the 
module when there's a device construct in the compilation unit, regardless of 
whether we're compiling for host or device and whether the device construct is 
a target region or a declare target subroutine. I think that covers all main 
aspects of this patch.

I have also detected that there is a situation where this approach doesn't 
currently work, so I created an expected-fail test for unnamed block data. The 
issue is that there is no symbol created for those, so there's nowhere to 
store/find these flags. I have seen this happen for main programs as well, but 
I've not been able to create a reproducer for which that happened that 
contained device constructs. In my opinion, this edge case should be addressed 
as its own patch.




Comment at: flang/include/flang/Lower/OpenMP.h:16
 
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include 

kiranchandramohan wrote:
> Is this include required here?
It was, but only because I had made public a function that wasn't supposed to 
be, thanks for noticing.



Comment at: flang/lib/Lower/Bridge.cpp:4779
+if (ompDeviceCodeFound)
+  Fortran::lower::genOpenMPRequires(getModuleOp().getOperation(),
+globalOmpRequiresSymbol);

kiranchandramohan wrote:
> If this is specific for device code, might be worth renaming it to something 
> specific to device.
This is for both host and device, it's just that it's only generated if there 
are device constructs in the compilation unit. Let me know if you still suggest 
renaming it to something else.

I modified the unit tests to run them for the target device as well, so that 
it's clear that the same behavior is expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 556542.
skatrak marked 3 inline comments as done.
skatrak added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
  flang/test/Lower/OpenMP/requires-common.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+  !$omp target
+  !$omp end target
+end program requires
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-common.f90
@@ -0,0 +1,19 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+block data init
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/Todo/requires-unnamed-common.f90
@@ -0,0 +1,25 @@
+! This test checks the lowering of REQUIRES inside of an unnamed BLOCK DATA.
+! The symbol of the `symTab` scope of the `BlockDataUnit` PFT node is null in
+! this case, resulting in the inability to store the REQUIRES flags gathered in
+! it.
+
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck %s
+! XFAIL: *
+
+! The string "EXPECTED" denotes the expected FIR
+
+!CHECK: module attributes {
+!CHECK-SAME:omp.requires = #omp
+block data
+  !$omp requires unified_shared_memory
+  integer :: x
+  common /block/ x
+  data x / 10 /
+end
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -78,9 +78,7 @@
 static void gatherFuncAndVarSyms(
 const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause,
-llvm::SmallVectorImpl>
-) {
+llvm::SmallVectorImpl ) {
   for (const Fortran::parser::OmpObject  : objList.v) {
 Fortran::common::visit(
 Fortran::common::visitors{
@@ -2474,6 +2472,71 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Could you add to the summary that the `atomic` related handling is done 
elsewhere.

Could you expand the tests to cover the various `if` conditions that are used 
in the code?




Comment at: flang/include/flang/Lower/OpenMP.h:16
 
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include 

Is this include required here?



Comment at: flang/lib/Lower/Bridge.cpp:315
+ [&](Fortran::lower::pft::BlockDataUnit ) {
+   if (!globalOmpRequiresSymbol)
+ globalOmpRequiresSymbol = b.symTab.symbol();

Is this handling required for `Block Data`? If so, could you add a test?



Comment at: flang/lib/Lower/Bridge.cpp:4779
+if (ompDeviceCodeFound)
+  Fortran::lower::genOpenMPRequires(getModuleOp().getOperation(),
+globalOmpRequiresSymbol);

If this is specific for device code, might be worth renaming it to something 
specific to device.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 556436.
skatrak added a comment.

Update patch. Ping for review and unblocking remaining accepted REQUIRES 
patches, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2473,6 +2473,47 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  mlir::omp::DeclareTargetDeviceType::any;
+  const auto  = std::get(
+  declareTargetConstruct.t);
+
+  if (const auto *objectList{
+  Fortran::parser::Unwrap(spec.u)}) {
+// Case: declare target(func, var1, var2)
+gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
+ symbolAndClause);
+  } else if (const auto *clauseList{
+ Fortran::parser::Unwrap(
+ spec.u)}) {
+if (clauseList->v.empty()) {
+  // Case: declare target, implicit capture of function
+  symbolAndClause.emplace_back(
+  mlir::omp::DeclareTargetCaptureClause::to,
+  eval.getOwningProcedure()->getSubprogramSymbol());
+}
+
+ClauseProcessor cp(converter, *clauseList);
+cp.processTo(symbolAndClause);
+cp.processLink(symbolAndClause);
+cp.processDeviceType(deviceType);
+cp.processTODO(
+converter.getCurrentLocation(),
+llvm::omp::Directive::OMPD_declare_target);
+  }
+
+  return deviceType;
+}
+
 //===--===//
 // genOMP() Code generation helper functions
 //===--===//
@@ -3453,35 +3494,8 @@
) {
   llvm::SmallVector symbolAndClause;
   mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
-  // The default capture type
-  mlir::omp::DeclareTargetDeviceType deviceType =
-  mlir::omp::DeclareTargetDeviceType::any;
-  const auto  = std::get(
-  declareTargetConstruct.t);
-  if (const auto *objectList{
-  Fortran::parser::Unwrap(spec.u)}) {
-// Case: declare target(func, var1, var2)
-gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
- symbolAndClause);
-  } else if (const auto *clauseList{
- Fortran::parser::Unwrap(
- spec.u)}) {
-if (clauseList->v.empty()) {
-  // Case: declare target, implicit capture of function
-  symbolAndClause.emplace_back(
-  mlir::omp::DeclareTargetCaptureClause::to,
-  eval.getOwningProcedure()->getSubprogramSymbol());
-}
-
-ClauseProcessor cp(converter, *clauseList);
-cp.processTo(symbolAndClause);
-cp.processLink(symbolAndClause);
-cp.processDeviceType(deviceType);
-cp.processTODO(
-converter.getCurrentLocation(),
-llvm::omp::Directive::OMPD_declare_target);
-  }
+  mlir::omp::DeclareTargetDeviceType deviceType = 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 552411.
skatrak marked an inline comment as done.
skatrak added a comment.

Update patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2467,6 +2467,47 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  mlir::omp::DeclareTargetDeviceType::any;
+  const auto  = std::get(
+  declareTargetConstruct.t);
+
+  if (const auto *objectList{
+  Fortran::parser::Unwrap(spec.u)}) {
+// Case: declare target(func, var1, var2)
+gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
+ symbolAndClause);
+  } else if (const auto *clauseList{
+ Fortran::parser::Unwrap(
+ spec.u)}) {
+if (clauseList->v.empty()) {
+  // Case: declare target, implicit capture of function
+  symbolAndClause.emplace_back(
+  mlir::omp::DeclareTargetCaptureClause::to,
+  eval.getOwningProcedure()->getSubprogramSymbol());
+}
+
+ClauseProcessor cp(converter, *clauseList);
+cp.processTo(symbolAndClause);
+cp.processLink(symbolAndClause);
+cp.processDeviceType(deviceType);
+cp.processTODO(
+converter.getCurrentLocation(),
+llvm::omp::Directive::OMPD_declare_target);
+  }
+
+  return deviceType;
+}
+
 //===--===//
 // genOMP() Code generation helper functions
 //===--===//
@@ -3383,35 +3424,8 @@
) {
   llvm::SmallVector symbolAndClause;
   mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
-  // The default capture type
-  mlir::omp::DeclareTargetDeviceType deviceType =
-  mlir::omp::DeclareTargetDeviceType::any;
-  const auto  = std::get(
-  declareTargetConstruct.t);
-  if (const auto *objectList{
-  Fortran::parser::Unwrap(spec.u)}) {
-// Case: declare target(func, var1, var2)
-gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
- symbolAndClause);
-  } else if (const auto *clauseList{
- Fortran::parser::Unwrap(
- spec.u)}) {
-if (clauseList->v.empty()) {
-  // Case: declare target, implicit capture of function
-  symbolAndClause.emplace_back(
-  mlir::omp::DeclareTargetCaptureClause::to,
-  eval.getOwningProcedure()->getSubprogramSymbol());
-}
-
-ClauseProcessor cp(converter, *clauseList);
-cp.processTo(symbolAndClause);
-cp.processLink(symbolAndClause);
-cp.processDeviceType(deviceType);
-cp.processTODO(
-converter.getCurrentLocation(),
-llvm::omp::Directive::OMPD_declare_target);
-  }
+  mlir::omp::DeclareTargetDeviceType deviceType = getDeclareTargetInfo(
+  converter, eval, 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 551506.
skatrak added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2407,6 +2407,44 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  mlir::omp::DeclareTargetDeviceType::any;
+  const auto  = std::get(
+  declareTargetConstruct.t);
+
+  if (const auto *objectList{
+  Fortran::parser::Unwrap(spec.u)}) {
+// Case: declare target(func, var1, var2)
+gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
+ symbolAndClause);
+  } else if (const auto *clauseList{
+ Fortran::parser::Unwrap(
+ spec.u)}) {
+if (clauseList->v.empty()) {
+  // Case: declare target, implicit capture of function
+  symbolAndClause.emplace_back(
+  mlir::omp::DeclareTargetCaptureClause::to,
+  eval.getOwningProcedure()->getSubprogramSymbol());
+}
+
+ClauseProcessor cp(converter, *clauseList);
+cp.processTo(symbolAndClause);
+cp.processLink(symbolAndClause);
+cp.processDeviceType(deviceType);
+  }
+
+  return deviceType;
+}
+
 //===--===//
 // genOMP() Code generation helper functions
 //===--===//
@@ -3306,32 +3344,8 @@
) {
   llvm::SmallVector symbolAndClause;
   mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
-  // The default capture type
-  mlir::omp::DeclareTargetDeviceType deviceType =
-  mlir::omp::DeclareTargetDeviceType::any;
-  const auto  = std::get(
-  declareTargetConstruct.t);
-  if (const auto *objectList{
-  Fortran::parser::Unwrap(spec.u)}) {
-// Case: declare target(func, var1, var2)
-gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
- symbolAndClause);
-  } else if (const auto *clauseList{
- Fortran::parser::Unwrap(
- spec.u)}) {
-if (clauseList->v.empty()) {
-  // Case: declare target, implicit capture of function
-  symbolAndClause.emplace_back(
-  mlir::omp::DeclareTargetCaptureClause::to,
-  eval.getOwningProcedure()->getSubprogramSymbol());
-}
-
-ClauseProcessor cp(converter, *clauseList);
-cp.processTo(symbolAndClause);
-cp.processLink(symbolAndClause);
-cp.processDeviceType(deviceType);
-  }
+  mlir::omp::DeclareTargetDeviceType deviceType = getDeclareTargetInfo(
+  converter, eval, declareTargetConstruct, symbolAndClause);
 
   for (const DeclareTargetCapturePair  : symbolAndClause) {
 mlir::Operation *op = mod.lookupSymbol(
@@ -3435,6 +3449,28 @@
   ompConstruct.u);
 }
 
+void Fortran::lower::analyzeOpenMPDeclarativeConstruct(
+

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 551500.
skatrak added a comment.

Rebase and address conflicts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2407,6 +2407,44 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  mlir::omp::DeclareTargetDeviceType::any;
+  const auto  = std::get(
+  declareTargetConstruct.t);
+
+  if (const auto *objectList{
+  Fortran::parser::Unwrap(spec.u)}) {
+// Case: declare target(func, var1, var2)
+gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
+ symbolAndClause);
+  } else if (const auto *clauseList{
+ Fortran::parser::Unwrap(
+ spec.u)}) {
+if (clauseList->v.empty()) {
+  // Case: declare target, implicit capture of function
+  symbolAndClause.emplace_back(
+  mlir::omp::DeclareTargetCaptureClause::to,
+  eval.getOwningProcedure()->getSubprogramSymbol());
+}
+
+ClauseProcessor cp(converter, *clauseList);
+cp.processTo(symbolAndClause);
+cp.processLink(symbolAndClause);
+cp.processDeviceType(deviceType);
+  }
+
+  return deviceType;
+}
+
 //===--===//
 // genOMP() Code generation helper functions
 //===--===//
@@ -3306,32 +3344,8 @@
) {
   llvm::SmallVector symbolAndClause;
   mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
-  // The default capture type
-  mlir::omp::DeclareTargetDeviceType deviceType =
-  mlir::omp::DeclareTargetDeviceType::any;
-  const auto  = std::get(
-  declareTargetConstruct.t);
-  if (const auto *objectList{
-  Fortran::parser::Unwrap(spec.u)}) {
-// Case: declare target(func, var1, var2)
-gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
- symbolAndClause);
-  } else if (const auto *clauseList{
- Fortran::parser::Unwrap(
- spec.u)}) {
-if (clauseList->v.empty()) {
-  // Case: declare target, implicit capture of function
-  symbolAndClause.emplace_back(
-  mlir::omp::DeclareTargetCaptureClause::to,
-  eval.getOwningProcedure()->getSubprogramSymbol());
-}
-
-ClauseProcessor cp(converter, *clauseList);
-cp.processTo(symbolAndClause);
-cp.processLink(symbolAndClause);
-cp.processDeviceType(deviceType);
-  }
+  mlir::omp::DeclareTargetDeviceType deviceType = getDeclareTargetInfo(
+  converter, eval, declareTargetConstruct, symbolAndClause);
 
   for (const DeclareTargetCapturePair  : symbolAndClause) {
 mlir::Operation *op = mod.lookupSymbol(
@@ -3435,6 +3449,28 @@
   ompConstruct.u);
 }
 
+void Fortran::lower::analyzeOpenMPDeclarativeConstruct(
+   

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 551113.
skatrak added a comment.

Change parent and update patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2407,6 +2407,44 @@
  reductionDeclSymbols));
 }
 
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::AbstractConverter ,
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+llvm::SmallVectorImpl ) {
+
+  // The default capture type
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  mlir::omp::DeclareTargetDeviceType::any;
+  const auto  = std::get(
+  declareTargetConstruct.t);
+
+  if (const auto *objectList{
+  Fortran::parser::Unwrap(spec.u)}) {
+// Case: declare target(func, var1, var2)
+gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
+ symbolAndClause);
+  } else if (const auto *clauseList{
+ Fortran::parser::Unwrap(
+ spec.u)}) {
+if (clauseList->v.empty()) {
+  // Case: declare target, implicit capture of function
+  symbolAndClause.emplace_back(
+  mlir::omp::DeclareTargetCaptureClause::to,
+  eval.getOwningProcedure()->getSubprogramSymbol());
+}
+
+ClauseProcessor cp(converter, *clauseList);
+cp.processTo(symbolAndClause);
+cp.processLink(symbolAndClause);
+cp.processDeviceType(deviceType);
+  }
+
+  return deviceType;
+}
+
 //===--===//
 // genOMP() Code generation helper functions
 //===--===//
@@ -3306,32 +3344,8 @@
) {
   llvm::SmallVector symbolAndClause;
   mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
-  // The default capture type
-  mlir::omp::DeclareTargetDeviceType deviceType =
-  mlir::omp::DeclareTargetDeviceType::any;
-  const auto  = std::get(
-  declareTargetConstruct.t);
-  if (const auto *objectList{
-  Fortran::parser::Unwrap(spec.u)}) {
-// Case: declare target(func, var1, var2)
-gatherFuncAndVarSyms(*objectList, mlir::omp::DeclareTargetCaptureClause::to,
- symbolAndClause);
-  } else if (const auto *clauseList{
- Fortran::parser::Unwrap(
- spec.u)}) {
-if (clauseList->v.empty()) {
-  // Case: declare target, implicit capture of function
-  symbolAndClause.emplace_back(
-  mlir::omp::DeclareTargetCaptureClause::to,
-  eval.getOwningProcedure()->getSubprogramSymbol());
-}
-
-ClauseProcessor cp(converter, *clauseList);
-cp.processTo(symbolAndClause);
-cp.processLink(symbolAndClause);
-cp.processDeviceType(deviceType);
-  }
+  mlir::omp::DeclareTargetDeviceType deviceType = getDeclareTargetInfo(
+  converter, eval, declareTargetConstruct, symbolAndClause);
 
   for (const DeclareTargetCapturePair  : symbolAndClause) {
 mlir::Operation *op = mod.lookupSymbol(
@@ -3435,6 +3449,28 @@
   ompConstruct.u);
 }
 
+void Fortran::lower::analyzeOpenMPDeclarativeConstruct(
+ 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 534512.
skatrak added a comment.

Update to integrate with changes to parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2676,16 +2676,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
-  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const Fortran::parser::OmpObject  : objList.v) {
@@ -2710,6 +2708,7 @@
   Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto  = std::get(
   declareTargetConstruct.t);
+
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2744,6 +2743,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void genDeclareTarget(Fortran::lower::AbstractConverter ,
+  Fortran::lower::pft::Evaluation ,
+  const Fortran::parser::OpenMPDeclareTargetConstruct
+  ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
   for (std::pair
symClause : symbolAndClause) {
@@ -2770,35 +2791,44 @@
   converter.getCurrentLocation(),
   "Attempt to apply declare target on unsupported operation");
 
-mlir::omp::DeclareTargetDeviceType newDeviceType;
-switch (deviceType) {
-case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::nohost;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Host:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::host;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Any:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::any;
-  break;
-}
-
 // The function or global already has a declare target applied to it,
 // very likely through implicit capture (usage in another declare
 // target function/subroutine). It should be marked as any if it has
 // been assigned both host and 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 531717.
skatrak added a comment.

Rebasde to fix build error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2603,16 +2603,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
-  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const Fortran::parser::OmpObject  : objList.v) {
@@ -2637,6 +2635,7 @@
   Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto  = std::get(
   declareTargetConstruct.t);
+
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2671,6 +2670,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void genDeclareTarget(Fortran::lower::AbstractConverter ,
+  Fortran::lower::pft::Evaluation ,
+  const Fortran::parser::OpenMPDeclareTargetConstruct
+  ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
   for (std::pair
symClause : symbolAndClause) {
@@ -2697,35 +2718,44 @@
   converter.getCurrentLocation(),
   "Attempt to apply declare target on unsupported operation");
 
-mlir::omp::DeclareTargetDeviceType newDeviceType;
-switch (deviceType) {
-case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::nohost;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Host:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::host;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Any:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::any;
-  break;
-}
-
 // The function or global already has a declare target applied to it,
 // very likely through implicit capture (usage in another declare
 // target function/subroutine). It should be marked as any if it has
 // been assigned both host and nohost, else we skip, as 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak requested review of this revision.
skatrak added a comment.

Changing status to "Request Review" again due to some significant changes to 
the approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 531437.
skatrak added a comment.

Update patch to integrate with related patch D149337 
 and address reviewer's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2594,16 +2594,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
-  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
-
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const Fortran::parser::OmpObject  : objList.v) {
@@ -2628,6 +2626,7 @@
   Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto  = std::get(
   declareTargetConstruct.t);
+
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2662,6 +2661,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void genDeclareTarget(Fortran::lower::AbstractConverter ,
+  Fortran::lower::pft::Evaluation ,
+  const Fortran::parser::OpenMPDeclareTargetConstruct
+  ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
   for (std::pair
symClause : symbolAndClause) {
@@ -2688,35 +2709,44 @@
   converter.getCurrentLocation(),
   "Attempt to apply declare target on unsupported operation");
 
-mlir::omp::DeclareTargetDeviceType newDeviceType;
-switch (deviceType) {
-case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::nohost;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Host:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::host;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Any:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::any;
-  break;
-}
-
 // The function or global already has a declare target applied to it,
 // very likely through implicit capture (usage in another declare
 // target function/subroutine). It 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak added a comment.

Thank you for the feedback. I expect I will be able to refactor the 
`analyzeOpenMPDeclarative()` function to OpenMP.cpp as suggested, but I'll 
bring that together with other changes to make it integrate cleanly with the 
latest changes to D149337 . Since that patch 
now moves a big part of the gathering of 'requires' clauses to the prior 
semantics step, this patch can be simplified to rely on that.




Comment at: flang/lib/Lower/Bridge.cpp:271
+bridge{bridge}, foldingContext{bridge.createFoldingContext()},
+ompRequiresFlags{mlir::omp::ClauseRequires::none} {}
   virtual ~FirConverter() = default;

kiranchandramohan wrote:
> Is this set anywhere? If not is it required? Can this be made available 
> through the bridge?
The `ompRequiresFlags` field is set by the `analyzeOpenMPDeclarative` method, 
called for each 'omp requires' directive in the PFT.

I'm not sure if I understand what you mean by making this available through the 
bridge. Do you suggest creating a public method to access these flags?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.



Comment at: flang/lib/Lower/Bridge.cpp:271
+bridge{bridge}, foldingContext{bridge.createFoldingContext()},
+ompRequiresFlags{mlir::omp::ClauseRequires::none} {}
   virtual ~FirConverter() = default;

Is this set anywhere? If not is it required? Can this be made available through 
the bridge?



Comment at: flang/lib/Lower/Bridge.cpp:2234
+  /// Extract information from OpenMP declarative constructs
+  void analyzeOpenMPDeclarative(
+  const Fortran::parser::OpenMPDeclarativeConstruct ) {

Can this function be moved to OpenMP.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Akash Banerjee via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak marked 2 inline comments as done.
skatrak added a comment.

Thank you for the comments, they should have all been addressed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 527354.
skatrak added a comment.

Rebase and address reviewer's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -2494,14 +2494,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const auto  : objList.v) {
@@ -2521,12 +2521,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType{Fortran::parser::OmpDeviceTypeClause::Type::Any};
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2562,6 +2561,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void genDeclareTarget(Fortran::lower::AbstractConverter ,
+  Fortran::lower::pft::Evaluation ,
+  const Fortran::parser::OpenMPDeclareTargetConstruct
+  ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  mlir::omp::DeclareTargetDeviceType deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
+  mlir::ModuleOp mod = converter.getFirOpBuilder().getModule();
   for (auto sym : symbolAndClause) {
 auto *op = mod.lookupSymbol(converter.mangleName(std::get<1>(sym)));
 
@@ -2571,32 +2592,19 @@
   converter.getCurrentLocation(),
   "Attempt to apply declare target on unsupproted operation");
 
-mlir::omp::DeclareTargetDeviceType newDeviceType;
-switch (deviceType) {
-case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::nohost;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Host:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::host;
-  break;
-case Fortran::parser::OmpDeviceTypeClause::Type::Any:
-  newDeviceType = mlir::omp::DeclareTargetDeviceType::any;
-  break;
-}
-
 // The function or global already has a declare target applied to it,
 // very 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added inline comments.



Comment at: flang/lib/Lower/OpenMP.cpp:2552
+
+void handleDeclareTarget(Fortran::lower::AbstractConverter ,
+ Fortran::lower::pft::Evaluation ,

This seems to be the only function name with the 'handle' prefix. Maybe rename 
it to something like `genDeclareTarget` ?



Comment at: flang/lib/Lower/OpenMP.cpp:2563
+
+  auto mod = converter.getFirOpBuilder().getModule();
   for (auto sym : symbolAndClause) {

Remove use of auto here.

Also in other places if the type isn't immediately discernible then consider 
naming the type. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

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

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 520398.
skatrak added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90
  mlir/lib/Dialect/OpenMP/CMakeLists.txt

Index: mlir/lib/Dialect/OpenMP/CMakeLists.txt
===
--- mlir/lib/Dialect/OpenMP/CMakeLists.txt
+++ mlir/lib/Dialect/OpenMP/CMakeLists.txt
@@ -12,4 +12,5 @@
   LINK_LIBS PUBLIC
   MLIRIR
   MLIRLLVMDialect
+  MLIRFuncDialect
   )
Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -23,7 +23,10 @@
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2469,14 +2472,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const auto  : objList.v) {
@@ -2496,12 +2499,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2537,6 +2539,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void handleDeclareTarget(Fortran::lower::AbstractConverter ,
+ Fortran::lower::pft::Evaluation ,
+ const Fortran::parser::OpenMPDeclareTargetConstruct
+ ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  auto deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
+  auto mod = converter.getFirOpBuilder().getModule();
   for (auto sym : symbolAndClause) {
 auto *op = mod.lookupSymbol(converter.mangleName(std::get<1>(sym)));
 
@@ -2546,32 +2570,19 @@
   converter.getCurrentLocation(),
   "Attempt to apply declare target on unsupproted operation");
 
-

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 520337.
skatrak added a comment.

Remove handling of `atomic_default_mem_order`, as it's now done in semantics. 
Fix conflicts with changes to parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90
  mlir/lib/Dialect/OpenMP/CMakeLists.txt

Index: mlir/lib/Dialect/OpenMP/CMakeLists.txt
===
--- mlir/lib/Dialect/OpenMP/CMakeLists.txt
+++ mlir/lib/Dialect/OpenMP/CMakeLists.txt
@@ -12,4 +12,5 @@
   LINK_LIBS PUBLIC
   MLIRIR
   MLIRLLVMDialect
+  MLIRFuncDialect
   )
Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,11 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2262,14 +2264,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const auto  : objList.v) {
@@ -2289,12 +2291,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2330,6 +2331,28 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void handleDeclareTarget(Fortran::lower::AbstractConverter ,
+ Fortran::lower::pft::Evaluation ,
+ const Fortran::parser::OpenMPDeclareTargetConstruct
+ ) {
+  llvm::SmallVector,
+0>
+  symbolAndClause;
+  auto deviceType =
+  getDeclareTargetInfo(eval, declareTargetConstruct, symbolAndClause);
+
+  auto mod = converter.getFirOpBuilder().getModule();
   for (auto sym : symbolAndClause) {
 auto *op = mod.lookupSymbol(converter.mangleName(std::get<1>(sym)));
 
@@ -2339,32 +2362,19 @@
   converter.getCurrentLocation(),
   "Attempt to 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 518698.
skatrak added a comment.

Rebase. Disable some known failing 'declare target' tests temporarily until 
they are fixed by parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/omp-declare-target-data.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/omp-declare-target-data.f90
===
--- flang/test/Lower/OpenMP/omp-declare-target-data.f90
+++ flang/test/Lower/OpenMP/omp-declare-target-data.f90
@@ -1,7 +1,8 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s 
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s
-!RUN: %flang_fc1 -emit-llvm-bc -fopenmp -o %t.bc %s | llvm-dis %t.bc -o - | FileCheck %s --check-prefix=HOST
-!RUN: %flang_fc1 -emit-llvm -fopenmp -fopenmp-is-device -fopenmp-host-ir-file-path %t.bc -o - %s 2>&1 | FileCheck %s --check-prefix=DEVICE
+!COM: TODO Uncomment following commands, once all required declare target lowering work lands
+!COM: %flang_fc1 -emit-llvm-bc -fopenmp -o %t.bc %s | llvm-dis %t.bc -o - | FileCheck %s --check-prefix=HOST
+!COM: %flang_fc1 -emit-llvm -fopenmp -fopenmp-is-device -fopenmp-host-ir-file-path %t.bc -o - %s 2>&1 | FileCheck %s --check-prefix=DEVICE
 
 !HOST-DAG: %struct.__tgt_offload_entry = type { ptr, ptr, i64, i32, i32 }
 !HOST-DAG: !omp_offload.info = !{!{{.*}}}
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2262,14 +2264,14 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector,
-0>
-  symbolAndClause;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl> ) {
+  // Gather the symbols and clauses
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ,
 mlir::omp::DeclareTargetCaptureClause clause) {
 for (const auto  : objList.v) {
@@ -2289,12 +2291,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2330,22 +2331,31 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 517844.
skatrak added a comment.

Remove leftover unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2239,11 +2241,13 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector symbols;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl ) {
+  // Gather the symbols
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ) {
 for (const auto  : objList.v) {
   Fortran::common::visit(
@@ -2261,12 +2265,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2288,8 +2291,7 @@
  std::get_if(
  )}) {
 // Case: declare target link(var1, var2)...
-TODO(converter.getCurrentLocation(),
- "the link clause is currently unsupported");
+TODO_NOLOC("the link clause is currently unsupported");
   } else if (const auto *deviceClause{
  std::get_if(
  )}) {
@@ -2299,6 +2301,24 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Nohost:
+return mlir::omp::DeclareTargetDeviceType::nohost;
+  }
+}
+
+void handleDeclareTarget(Fortran::lower::AbstractConverter ,
+ Fortran::lower::pft::Evaluation ,
+ const Fortran::parser::OpenMPDeclareTargetConstruct
+ ) {
+  SmallVector symbols;
+  auto deviceType = getDeclareTargetInfo(eval, declareTargetConstruct, symbols);
+
+  auto mod = converter.getFirOpBuilder().getModule();
   for (auto sym : symbols) {
 auto *op = mod.lookupSymbol(converter.mangleName(sym));
 
@@ -2309,20 +2329,6 @@
   TODO(converter.getCurrentLocation(),
"only subroutines and functions are currently supported");
 
-mlir::omp::DeclareTargetDeviceType 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 517616.
skatrak added a comment.

Remove checks from lowering that belong in semantic analysis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-error.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-error.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-error.f90
@@ -0,0 +1,15 @@
+! RUN: not %flang_fc1 -emit-fir -fopenmp %s -o - 2>&1 | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  error: {{.*}} conflicting atomic_default_mem_order clause found:
+!CHECK-SAME: acq_rel != seq_cst
+program requires
+  !$omp requires atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f()
+  !$omp requires atomic_default_mem_order(acq_rel)
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2239,11 +2241,13 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector symbols;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl ) {
+  // Gather the symbols
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ) {
 for (const auto  : objList.v) {
   Fortran::common::visit(
@@ -2261,12 +2265,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2288,8 +2291,7 @@
  std::get_if(
  )}) {
 // Case: declare target link(var1, var2)...
-TODO(converter.getCurrentLocation(),
- "the link clause is currently unsupported");
+TODO_NOLOC("the link clause is currently unsupported");
   } else if (const auto *deviceClause{
  std::get_if(
  )}) {
@@ -2299,6 +2301,24 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return 

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 516730.
skatrak added a comment.

Try undoing last change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-error.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-error.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-error.f90
@@ -0,0 +1,15 @@
+! RUN: not %flang_fc1 -emit-fir -fopenmp %s -o - 2>&1 | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  error: {{.*}} conflicting atomic_default_mem_order clause found:
+!CHECK-SAME: acq_rel != seq_cst
+program requires
+  !$omp requires atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f()
+  !$omp requires atomic_default_mem_order(acq_rel)
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2239,11 +2241,13 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter ,
- Fortran::lower::pft::Evaluation ,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- ) {
-  llvm::SmallVector symbols;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation ,
+const Fortran::parser::OpenMPDeclareTargetConstruct ,
+SmallVectorImpl ) {
+  // Gather the symbols
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList ) {
 for (const auto  : objList.v) {
   Fortran::common::visit(
@@ -2261,12 +2265,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto {std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2288,8 +2291,7 @@
  std::get_if(
  )}) {
 // Case: declare target link(var1, var2)...
-TODO(converter.getCurrentLocation(),
- "the link clause is currently unsupported");
+TODO_NOLOC("the link clause is currently unsupported");
   } else if (const auto *deviceClause{
  std::get_if(
  )}) {
@@ -2299,6 +2301,24 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+  case Fortran::parser::OmpDeviceTypeClause::Type::Host:
+return mlir::omp::DeclareTargetDeviceType::host;
+  

[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 516729.
skatrak added a comment.
Herald added subscribers: llvm-commits, cfe-commits, bviyer, Moerafaat, 
zero9178, awarzynski, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, 
mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, thopre, 
hiraditya.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: ftynse.
Herald added projects: clang, MLIR, LLVM.

Try to fix issue with applying patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/lib/Semantics/CMakeLists.txt
  flang/lib/Semantics/finalize-omp.cpp
  flang/lib/Semantics/finalize-omp.h
  flang/lib/Semantics/semantics.cpp
  flang/test/Lower/OpenMP/Todo/omp-declare-target.f90
  flang/test/Lower/OpenMP/omp-declare-target.f90
  flang/test/Lower/OpenMP/requires-error.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90
  flang/test/Semantics/OpenMP/declare-target-implicit-capture-rewrite.f90
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
  mlir/lib/Target/LLVMIR/CMakeLists.txt
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
  mlir/test/Dialect/OpenMP/attr.mlir
  mlir/test/Target/LLVMIR/openmp-llvm.mlir

Index: mlir/test/Target/LLVMIR/openmp-llvm.mlir
===
--- mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -2537,3 +2537,51 @@
 // CHECK: @__omp_rtl_assume_no_nested_parallelism = weak_odr hidden constant i32 0
 module attributes {omp.flags = #omp.flags, 
omp.is_device = #omp.isdevice} {}
+
+// -
+
+// Check that the atomic default memory order is picked up by atomic operations.
+module attributes {
+  omp.atomic_default_mem_order = #omp
+} {
+  // CHECK-LABEL: @omp_atomic_default_mem_order
+  // CHECK-SAME: (ptr %[[ARG0:.*]], ptr %[[ARG1:.*]], i32 %[[EXPR:.*]])
+  llvm.func @omp_atomic_default_mem_order(%arg0 : !llvm.ptr,
+  %arg1 : !llvm.ptr,
+  %expr : i32) -> () {
+
+// CHECK: %[[X1:.*]] = load atomic i32, ptr %[[ARG0]] seq_cst, align 4
+// CHECK: store i32 %[[X1]], ptr %[[ARG1]], align 4
+omp.atomic.read %arg1 = %arg0 : !llvm.ptr, i32
+
+// CHECK: store atomic i32 %[[EXPR]], ptr %[[ARG1]] seq_cst, align 4
+// CHECK: call void @__kmpc_flush(ptr @{{.*}})
+omp.atomic.write %arg1 = %expr : !llvm.ptr, i32
+
+// CHECK: atomicrmw add ptr %[[ARG1]], i32 %[[EXPR]] seq_cst
+omp.atomic.update %arg1 : !llvm.ptr {
+^bb0(%xval: i32):
+  %newval = llvm.add %xval, %expr : i32
+  omp.yield(%newval : i32)
+}
+
+// CHECK: %[[xval:.*]] = atomicrmw xchg ptr %[[ARG0]], i32 %[[EXPR]] seq_cst
+// CHECK: store i32 %[[xval]], ptr %[[ARG1]]
+omp.atomic.capture {
+  omp.atomic.read %arg1 = %arg0 : !llvm.ptr, i32
+  omp.atomic.write %arg0 = %expr : !llvm.ptr, i32
+}
+
+llvm.return
+  }
+}
+
+// -
+
+// Check that OpenMP requires flags are registered by a global constructor.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }]
+// CHECK-SAME: [{ i32, ptr, ptr } { i32 0, ptr @[[REG_FN:.*]], ptr null }]
+// CHECK: define {{.*}} @[[REG_FN]]({{.*}})
+// CHECK-NOT: }
+// CHECK:   call void @__tgt_register_requires(i64 10)
+module attributes {omp.requires = #omp} {}
Index: mlir/test/Dialect/OpenMP/attr.mlir
===
--- mlir/test/Dialect/OpenMP/attr.mlir
+++ mlir/test/Dialect/OpenMP/attr.mlir
@@ -29,3 +29,23 @@
 
 // CHECK: module attributes {omp.flags = #omp.flags} {
 module attributes {omp.flags = #omp.flags} {}
+
+// 
+
+// CHECK-LABEL: func @omp_decl_tar_host
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_host() -> () attributes {omp.declare_target = #omp} {
+  return
+}
+
+// CHECK-LABEL: func @omp_decl_tar_nohost
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_nohost() -> () attributes {omp.declare_target = #omp} {
+  return
+}
+
+// CHECK-LABEL: func @omp_decl_tar_any
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_any() -> () attributes {omp.declare_target =