[flang][OpenMP] Upstream first part of do concurrent mapping
#126026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-driver Author: Kareem Ergawy (ergawy) ChangesThis PR starts the effort to upstream AMD's internal implementation of An important part of this PR is a document that describes the current status downstream, the upstreaming status, and next steps to make this pass much more useful. In addition to this document, this PR also contains the skeleton of the pass (no useful transformations are done yet) and some testing for the added command line options. Patch is 36.89 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126026.diff 17 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c0749c418b7bcec..7fc22bcdff17cd2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6901,6 +6901,10 @@ defm loop_versioning : BoolOptionWithoutMarshalling<"f", "version-loops-for-stri
def fhermetic_module_files : Flag<["-"], "fhermetic-module-files">, Group<f_Group>,
HelpText<"Emit hermetic module files (no nested USE association)">;
+
+def do_concurrent_parallel_EQ : Joined<["-"], "fdo-concurrent-parallel=">,
+ HelpText<"Try to map `do concurrent` loops to OpenMP [none|host|device]">,
+ Values<"none,host,device">;
} // let Visibility = [FC1Option, FlangOption]
def J : JoinedOrSeparate<["-"], "J">,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index e4019c434968744..cf08ec1b900ad90 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -153,7 +153,8 @@ void Flang::addCodegenOptions(const ArgList &Args,
CmdArgs.push_back("-fversion-loops-for-stride");
Args.addAllArgs(CmdArgs,
- {options::OPT_flang_experimental_hlfir,
+ {options::OPT_do_concurrent_parallel_EQ,
+ options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_fno_ppc_native_vec_elem_order,
options::OPT_fppc_native_vec_elem_order,
diff --git a/flang/docs/DoConcurrentConversionToOpenMP.md b/flang/docs/DoConcurrentConversionToOpenMP.md
new file mode 100644
index 000000000000000..d40383a06a47b6c
--- /dev/null
+++ b/flang/docs/DoConcurrentConversionToOpenMP.md
@@ -0,0 +1,380 @@
+<!--===- docs/DoConcurrentMappingToOpenMP.md
+
+ Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ See https://llvm.org/LICENSE.txt for license information.
+ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+-->
+
+# `DO CONCURENT` mapping to OpenMP
+
+```{contents}
+---
+local:
+---
+```
+
+This document seeks to describe the effort to parallelize `do concurrent` loops
+by mapping them to OpenMP worksharing constructs. The goals of this document
+are:
+* Describing how to instruct `flang` to map `DO CONCURENT` loops to OpenMP
+ constructs.
+* Tracking the current status of such mapping.
+* Describing the limitations of the current implmenentation.
+* Describing next steps.
+* Tracking the current upstreaming status (from the AMD ROCm fork).
+
+## Usage
+
+In order to enable `do concurrent` to OpenMP mapping, `flang` adds a new
+compiler flag: `-fdo-concurrent-parallel`. This flags has 3 possible values:
+1. `host`: this maps `do concurent` loops to run in parallel on the host CPU.
+ This maps such loops to the equivalent of `omp parallel do`.
+2. `device`: this maps `do concurent` loops to run in parallel on a device
+ (GPU). This maps such loops to the equivalent of `omp target teams
+ distribute parallel do`.
+3. `none`: this disables `do concurrent` mapping altogether. In such case, such
+ loops are emitted as sequential loops.
+
+The above compiler switch is currently avaialble only when OpenMP is also
+enabled. So you need to provide the following options to flang in order to
+enable it:
+```
+flang ... -fopenmp -fdo-concurrent-parallel=[host|device|none] ...
+```
+
+## Current status
+
+Under the hood, `do concurrent` mapping is implemented in the
+`DoConcurrentConversionPass`. This is still an experimental pass which means
+that:
+* It has been tested in a very limited way so far.
+* It has been tested mostly on simple synthetic inputs.
+
+To describe current status in more detail, following is a description of how
+the pass currently behaves for single-range loops and then for multi-range
+loops. The following sub-sections describe the status of the downstream
+implementation on the AMD's ROCm fork(*). We are working on upstreaming the
+downstream implementation gradually and this document will be updated to reflect
+such upstreaming process. Example LIT tests referenced below might also be only
+be available in the ROCm fork and will upstream with the relevant parts of the
+code.
+
+(*) https://github.com/ROCm/llvm-project/blob/amd-staging/flang/lib/Optimizer/OpenMP/DoConcurrentConversion.cpp
+
+### Single-range loops
+
+Given the following loop:
+```fortran
+ do concurrent(i=1:n)
+ a(i) = i * i
+ end do
+```
+
+#### Mapping to `host`
+
+Mapping this loop to the `host`, generates MLIR operations of the following
+structure:
+
+```mlir
+%4 = fir.address_of(@_QFEa) ...
+%6:2 = hlfir.declare %4 ...
+
+omp.parallel {
+ // Allocate private copy for `i`.
+ // TODO Use delayed privatization.
+ %19 = fir.alloca i32 {bindc_name = "i"}
+ %20:2 = hlfir.declare %19 {uniq_name = "_QFEi"} ...
+
+ omp.wsloop {
+ omp.loop_nest (%arg0) : index = (%21) to (%22) inclusive step (%c1_2) {
+ %23 = fir.convert %arg0 : (index) -> i32
+ // Use the privatized version of `i`.
+ fir.store %23 to %20#1 : !fir.ref<i32>
+ ...
+
+ // Use "shared" SSA value of `a`.
+ %42 = hlfir.designate %6#0
+ hlfir.assign %35 to %42
+ ...
+ omp.yield
+ }
+ omp.terminator
+ }
+ omp.terminator
+}
+```
+
+#### Mapping to `device`
+
+Mapping the same loop to the `device`, generates MLIR operations of the
+following structure:
+
+```mlir
+// Map `a` to the `target` region. The pass automatically detects memory blocks
+// and maps them to device. Currently detection logic is still limited and a lot
+// of work is going into making it more capable.
+%29 = omp.map.info ... {name = "_QFEa"}
+omp.target ... map_entries(..., %29 -> %arg4 ...) {
+ ...
+ %51:2 = hlfir.declare %arg4
+ ...
+ omp.teams {
+ // Allocate private copy for `i`.
+ // TODO Use delayed privatization.
+ %52 = fir.alloca i32 {bindc_name = "i"}
+ %53:2 = hlfir.declare %52
+ ...
+
+ omp.parallel {
+ omp.distribute {
+ omp.wsloop {
+ omp.loop_nest (%arg5) : index = (%54) to (%55) inclusive step (%c1_9) {
+ // Use the privatized version of `i`.
+ %56 = fir.convert %arg5 : (index) -> i32
+ fir.store %56 to %53#1
+ ...
+ // Use the mapped version of `a`.
+ ... = hlfir.designate %51#0
+ ...
+ }
+ omp.terminator
+ }
+ omp.terminator
+ }
+ omp.terminator
+ }
+ omp.terminator
+ }
+ omp.terminator
+}
+```
+
+### Multi-range loops
+
+The pass currently supports multi-range loops as well. Given the following
+example:
+
+```fortran
+ do concurrent(i=1:n, j=1:m)
+ a(i,j) = i * j
+ end do
+```
+
+The generated `omp.loop_nest` operation look like:
+
+```mlir
+omp.loop_nest (%arg0, %arg1)
+ : index = (%17, %19) to (%18, %20)
+ inclusive step (%c1_2, %c1_4) {
+ fir.store %arg0 to %private_i#1 : !fir.ref<i32>
+ fir.store %arg1 to %private_j#1 : !fir.ref<i32>
+ ...
+ omp.yield
+}
+```
+
+It is worth noting that we have privatized versions for both iteration
+variables: `i` and `j`. These are locally allocated inside the parallel/target
+OpenMP region similar to what the single-range example in previous section
+shows.
+
+#### Multi-range and perfectly-nested loops
+
+Currently, on the `FIR` dialect level, the following loop:
+```fortran
+do concurrent(i=1:n, j=1:m)
+ a(i,j) = i * j
+end do
+```
+is modelled as a nest of `fir.do_loop` ops such that the outer loop's region
+contains:
+ 1. The operations needed to assign/update the outer loop's induction variable.
+ 1. The inner loop itself.
+
+So the MLIR structure looks similar to the following:
+```mlir
+fir.do_loop %arg0 = %11 to %12 step %c1 unordered {
+ ...
+ fir.do_loop %arg1 = %14 to %15 step %c1_1 unordered {
+ ...
+ }
+}
+```
+This applies to multi-range loops in general; they are represented in the IR as
+a nest of `fir.do_loop` ops with the above nesting structure.
+
+Therefore, the pass detects such "perfectly" nested loop ops to identify multi-range
+loops and map them as "collapsed" loops in OpenMP.
+
+#### Further info regarding loop nest detection
+
+Loop-nest detection is currently limited to the scenario described in the previous
+section. However, this is quite limited and can be extended in the future to cover
+more cases. For example, for the following loop nest, even thought, both loops are
+perfectly nested; at the moment, only the outer loop is parallized:
+```fortran
+do concurrent(i=1:n)
+ do concurrent(j=1:m)
+ a(i,j) = i * j
+ end do
+end do
+```
+
+Similary for the following loop nest, even though the intervening statement `x = 41`
+does not have any memory effects that would affect parallization, this nest is
+not parallized as well (only the outer loop is).
+
+```fortran
+do concurrent(i=1:n)
+ x = 41
+ do concurrent(j=1:m)
+ a(i,j) = i * j
+ end do
+end do
+```
+
+The above also has the consequence that the `j` variable will **not** be
+privatized in the OpenMP parallel/target region. In other words, it will be
+treated as if it was a `shared` variable. For more details about privatization,
+see the "Data environment" section below.
+
+See `flang/test/Transforms/DoConcurrent/loop_nest_test.f90` for more examples
+of what is and is not detected as a perfect loop nest.
+
+### Data environment
+
+By default, variables that are used inside a `do concurernt` loop nest are
+either treated as `shared` in case of mapping to `host`, or mapped into the
+`target` region using a `map` clause in case of mapping to `device`. The only
+exceptions to this are:
+ 1. the loop's iteration variable(s) (IV) of **perfect** loop nests. In that
+ case, for each IV, we allocate a local copy as shown the by the mapping
+ examples above.
+ 1. any values that are from allocations outside the loop nest and used
+ exclusively inside of it. In such cases, a local privatized
+ value is created in the OpenMP region to prevent multiple teams of threads
+ from accessing and destroying the same memory block which causes runtime
+ issues. For an example of such cases, see
+ `flang/test/Transforms/DoConcurrent/locally_destroyed_temp.f90`.
+
+Implicit mapping detection (for mapping to the GPU) is still quite limited and
+work to make it smarter is underway for both OpenMP in general and `do concurrent`
+mapping.
+
+#### Non-perfectly-nested loops' IVs
+
+For non-perfectly-nested loops, the IVs are still treated as `shared` or
+`map` entries as pointed out above. This **might not** be consistent with what
+the Fortran specficiation tells us. In particular, taking the following
+snippets from the spec (version 2023) into account:
+
+> § 3.35
+> ------
+> construct entity
+> entity whose identifier has the scope of a construct
+
+> § 19.4
+> ------
+> A variable that appears as an index-name in a FORALL or DO CONCURRENT
+> construct, or ... is a construct entity. A variable that has LOCAL or
+> LOCAL_INIT locality in a DO CONCURRENT construct is a construct entity.
+> ...
+> The name of a variable that appears as an index-name in a DO CONCURRENT
+> construct, FORALL statement, or FORALL construct has a scope of the statement
+> or construct. A variable that has LOCAL or LOCAL_INIT locality in a DO
+> CONCURRENT construct has the scope of that construct.
+
+From the above quotes, it seems there is an equivalence between the IV of a `do
+concurrent` loop and a variable with a `LOCAL` locality specifier (equivalent
+to OpenMP's `private` clause). Which means that we should probably
+localize/privatize a `do concurernt` loop's IV even if it is not perfectly
+nested in the nest we are parallelizing. For now, however, we **do not** do
+that as pointed out previously. In the near future, we propose a middle-ground
+solution (see the Next steps section for more details).
+
+## Next steps
+
+### Delayed privatization
+
+So far, we emit the privatization logic for IVs inline in the parallel/target
+region. This is enough for our purposes right now since we don't
+localize/privatize any sophisticated types of variables yet. Once we have need
+for more advanced localization through `do concurrent`'s locality specifiers
+(see below), delayed privatization will enable us to have a much cleaner IR.
+Once delayed privatization's implementation upstream is supported for the
+required constructs by the pass, we will move to it rather than inlined/early
+privatization.
+
+### Locality specifiers for `do concurrent`
+
+Locality specifiers will enable the user to control the data environment of the
+loop nest in a more fine-grained way. Implementing these specifiers on the
+`FIR` dialect level is needed in order to support this in the
+`DoConcurrentConversionPass`.
+
+Such specifiers will also unlock a potential solution to the
+non-perfectly-nested loops' IVs issue described above. In particular, for a
+non-perfectly nested loop, one middle-ground proposal/solution would be to:
+* Emit the loop's IV as shared/mapped just like we do currently.
+* Emit a warning that the IV of the loop is emitted as shared/mapped.
+* Given support for `LOCAL`, we can recommend the user to explicitly
+ localize/privatize the loop's IV if they choose to.
+
+#### Sharing TableGen clause records from the OpenMP dialect
+
+At the moment, the FIR dialect does not have a way to model locality specifiers
+on the IR level. Instead, something similar to early/eager privatization in OpenMP
+is done for the locality specifiers in `fir.do_loop` ops. Having locality specifier
+modelled in a way similar to delayed privatization (i.e. the `omp.private` op) and
+reductions (i.e. the `omp.delcare_reduction` op) can make mapping `do concurrent`
+to OpenMP (and other parallization models) much easier.
+
+Therefore, one way to approach this problem is to extract the TableGen records
+for relevant OpenMP clauses in a shared dialect for "data environment management"
+and use these shared records for OpenMP, `do concurrent`, and possibly OpenACC
+as well.
+
+### More advanced detection of loop nests
+
+As pointed out earlier, any intervening code between the headers of 2 nested
+`do concurrent` loops prevents us currently from detecting this as a loop nest.
+In some cases this is overly conservative. Therefore, a more flexible detection
+logic of loop nests needs to be implemented.
+
+### Data-dependence analysis
+
+Right now, we map loop nests without analysing whether such mapping is safe to
+do or not. We probalby need to at least warn the use of unsafe loop nests due
+to loop-carried dependencies.
+
+### Non-rectangular loop nests
+
+So far, we did not need to use the pass for non-rectangular loop nests. For
+example:
+```fortran
+do concurrent(i=1:n)
+ do concurrent(j=i:n)
+ ...
+ end do
+end do
+```
+We defer this to the (hopefully) near future when we get the conversion in a
+good share for the samples/projects at hand.
+
+### Generalizing the pass to other parallization models
+
+Once we have a stable and capable `do concurrent` to OpenMP mapping, we can take
+this in a more generalized direction and allow the pass to target other models;
+e.g. OpenACC. This goal should be kept in mind from the get-go even while only
+targeting OpenMP.
+
+
+## Upstreaming status
+
+- [x] Command line options for `flang` and `bbc`.
+- [x] Conversion pass skeleton (no transormations happen yet).
+- [x] Status description and tracking document (this document).
+- [ ] Basic host/CPU mapping support.
+- [ ] Basic device/GPU mapping support.
+- [ ] More advanced host and device support (expaned to multiple items as needed).
diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def
index deb8d1aede518b3..13cda965600b501 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.def
+++ b/flang/include/flang/Frontend/CodeGenOptions.def
@@ -41,5 +41,7 @@ ENUM_CODEGENOPT(DebugInfo, llvm::codegenoptions::DebugInfoKind, 4, llvm::codeg
ENUM_CODEGENOPT(VecLib, llvm::driver::VectorLibrary, 3, llvm::driver::VectorLibrary::NoLibrary) ///< Vector functions library to use
ENUM_CODEGENOPT(FramePointer, llvm::FramePointerKind, 2, llvm::FramePointerKind::None) ///< Enable the usage of frame pointers
+ENUM_CODEGENOPT(DoConcurrentMapping, DoConcurrentMappingKind, 2, DoConcurrentMappingKind::DCMK_None) ///< Map `do concurrent` to OpenMP
+
#undef CODEGENOPT
#undef ENUM_CODEGENOPT
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
index f19943335737b91..23d99e1f0897a0f 100644
--- a/flang/include/flang/Frontend/CodeGenOptions.h
+++ b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -15,6 +15,7 @@
#ifndef FORTRAN_FRONTEND_CODEGENOPTIONS_H
#define FORTRAN_FRONTEND_CODEGENOPTIONS_H
+#include "flang/Optimizer/OpenMP/Utils.h"
#include "llvm/Frontend/Debug/Options.h"
#include "llvm/Frontend/Driver/CodeGenOptions.h"
#include "llvm/Support/CodeGen.h"
@@ -143,6 +144,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
/// (-mlarge-data-threshold).
uint64_t LargeDataThreshold;
+ /// Optionally map `do concurrent` loops to OpenMP. This is only valid of
+ /// OpenMP is enabled.
+ using DoConcurrentMappingKind = flangomp::DoConcurrentMappingKind;
+
// Define accessors/mutators for code generation options of enumeration type.
#define CODEGENOPT(Name, Bits, Default)
#define ENUM_CODEGENOPT(Name, Type, Bits, Default) \
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.h b/flang/include/flang/Optimizer/OpenMP/Passes.h
index feb395f1a12dbd8..c67bddbcd270433 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.h
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.h
@@ -13,6 +13,7 @@
#ifndef FORTRAN_OPTIMIZER_OPENMP_PASSES_H
#define FORTRAN_OPTIMIZER_OPENMP_PASSES_H
+#include "flang/Optimizer/OpenMP/Utils.h"
#include "mlir/Dialect/Func/IR/FuncOps.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/Pass/Pass.h"
@@ -30,6 +31,7 @@ namespace flangomp {
/// divided into units of work.
bool shouldUseWorkshareLowering(mlir::Operation *op);
+std::unique_ptr<mlir::Pass> createDoConcurrentConversionPass(bool mapToDevice);
} // namespace flangomp
#endif // FORTRAN_OPTIMIZER_OPENMP_PASSES_H
diff --git a/flang/include/flang/Optimizer/OpenMP/Passes.td b/flang/include/flang/Optimizer/OpenMP/Passes.td
index 3add0c560f88dc9..b2152f8c674ea65 100644
--- a/flang/include/flang/Optimizer/OpenMP/Passes.td
+++ b/flang/include/flang/Optimizer/OpenMP/Passes.td
@@ -50,6 +50,36 @@ def FunctionFilteringPass : Pass<"omp-function-filtering"> {
];
}
+def DoConcurrentConversionPass : Pass<"fopenmp-do-concurrent-conversion", "mlir::func::FuncOp"> {
+ let summary = "Map `DO CONCURRENT` loops to OpenMP worksharing loops.";
+
+ let description = [{ This is an experimental pass to map `DO CONCURRENT` loops
+ to their correspnding equivalent OpenMP worksharing constructs.
+
+ For now the following is supported:
+ - Mapping simple loops to `parallel do`.
+
+ Still to TODO:
+ - More extensive testing.
+ }];
+
+ let dependentDialects = ["mlir::omp::OpenMPDialect"];
+
+ let options = [
+ Option<"mapTo", "map-to",
+ "flangomp::DoConcurrentMappingKind",
+ /*default=*/"flangomp::DoConcurrentMappingKind::DCMK_None",
+ "Try to map `do concurrent` loops to OpenMP [none|host|device]",
+ [{::llvm::cl::values(
+ clEnumValN(flangomp::DoConcurrentMappingKind::DCMK_None,
+ "none", "Do not lower `do concurrent` to OpenMP"),
+ clEnumValN(flangomp::DoConcurrentMappingKind::DCMK_Host,
+ "host", "Lower to run in parallel on the CPU"),
+ clEnumValN(flangomp::DoConcurrentMappingKind::DCMK_Device,
+ "device", "Lower to run in parallel on the GPU")
+ )}]>,
+ ];
+}
// Needs to be scheduled on Module as we create functions in it
def LowerWorkshare : Pass<"lower-workshare", "::mlir::ModuleOp"> {
diff --git a/flang/include/flang/Optimizer/OpenMP/Utils.h b/flang/include/flang/Optimizer/OpenMP/Utils.h
new file mode 100644
index 000000000000000..636c768b016b77f
--- /dev/null
+++ b/flang/include/flang/Optimizer/OpenMP/Utils.h
@@ -0,0 +1,26 @@
+//===-- Optimizer/OpenMP/Utils.h --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM ...
[truncated]
|
03d500e to
04b5656
Compare
clementval
reviewed
Feb 7, 2025
| @@ -6910,6 +6910,10 @@ defm loop_versioning : BoolOptionWithoutMarshalling<"f", "version-loops-for-stri | |||
|
|
|||
| def fhermetic_module_files : Flag<["-"], "fhermetic-module-files">, Group<f_Group>, | |||
| HelpText<"Emit hermetic module files (no nested USE association)">; | |||
|
|
|||
| def do_concurrent_parallel_EQ : Joined<["-"], "fdo-concurrent-parallel=">, | |||
There was a problem hiding this comment.
Can you make the option name reflect that this is targeting OpenMP?
There was a problem hiding this comment.
Great suggestion @clementval -- especially because there could someday be alternative options such as OpenACC for example.
There was a problem hiding this comment.
Yes there will be an OpenACC version of this.
There was a problem hiding this comment.
Changed to do-concurrent-to-openmp. Let me know if the name can be improved.
There was a problem hiding this comment.
Shall we have a single option -fdo-concurrent-parallel= and have the decision of whether to use OpenMP or OpenACC via -fopenmp and -fopenacc? -fdo-concurrent-parallel does not make much sense without one of the latter options. I guess it's a matter of proper documentation. If we have different flavors of this option, we would run into error cases that we'd have to handle such as -fopenacc -fdo-concurrent-to-openmp.
There was a problem hiding this comment.
PS: Does -fdo-concurrent-parallel= issue an error if actual parallelism is requested and -fopenmp is not present?
PPS: If we choose to rename the thing, then is should rather be -fopenmp-parallel-do-concurrent=, since this seems to be the common naming pattern for OpenMP-related compiler flags.
There was a problem hiding this comment.
I somewhat agree with @mjklemm regarding the name (that's why I didn't put openmp at the start). However, I will leave it for the reviewers. Also, this is something that I hope will be easy to fix retrospectively if we want to; let me know if you disagree.
PS: Does -fdo-concurrent-parallel= issue an error if actual parallelism is requested and -fopenmp is not present?
Yes, see changes in FrontendActions.cpp, an error message is emitted.
PPS: If we choose to rename the thing, then is should rather be -fopenmp-parallel-do-concurrent=, since this seems to be the common naming pattern for OpenMP-related compiler flags.
In my mind, this is a do concurrent feature/flag not an OpenMP one. It just happens to map to OpenMP atm.
clementval
reviewed
Feb 7, 2025
flang/tools/bbc/bbc.cpp
Outdated
| @@ -142,6 +142,12 @@ static llvm::cl::opt<bool> | |||
| llvm::cl::desc("enable openmp device compilation"), | |||
| llvm::cl::init(false)); | |||
|
|
|||
| static llvm::cl::opt<std::string> enableDoConcurrentToOpenMPConversion( | |||
| "fdo-concurrent-parallel", | |||
There was a problem hiding this comment.
Same here, the option name should probably have amp or smth that inform the user that this is targeting openmp under the hood.
04b5656 to
4ea5157
Compare
|
Ping! Please take a look when you have time. And let me know if you disagree with any decisions taken in the current approach or need any clarification/expansion in the next steps section of the status tracking document. |
skatrak
reviewed
Feb 11, 2025
There was a problem hiding this comment.
Thank you Kareem! I have some comments, though most of them are nits about typos and naming suggestions.
Also, I don't have a very strong opinion about this, but it seems misleading to have the DoConcurrentConversionToOpenMP.md document be written as if all of the features in the ROCm fork were already present, with the last section only implying that not everything that was presented is already there. It seems to me that it would make more sense that this document was updated with what's introduced by each patch, so it's easier to review together with the features as they come.
4ea5157 to
e24b6d2
Compare
|
Thanks for the patch. This is great work. The SCFToOpenMP pass introduced in https://reviews.llvm.org/D91982 might be another reference. It was extended to handle reductions. https://github.com/llvm/llvm-project/blob/main/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
+1. I agree with @skatrak here. I would also request the same unless it is too onerous. |
Comment on lines
214
to
224
| more cases. For example, for the following loop nest, even though, both loops are | ||
| perfectly nested; at the moment, only the outer loop is parallelized: | ||
| ```fortran | ||
| do concurrent(i=1:n) | ||
| do concurrent(j=1:m) | ||
| a(i,j) = i * j | ||
| end do | ||
| end do | ||
| ``` |
There was a problem hiding this comment.
Is that for multi-range loops as well?
There was a problem hiding this comment.
Luckily no. Mutli-range loops are collapsed. This is described in the section above.
Comment on lines
228
to
237
| ```fortran | ||
| do concurrent(i=1:n) | ||
| x = 41 | ||
| do concurrent(j=1:m) | ||
| a(i,j) = i * j | ||
| end do | ||
| end do | ||
| ``` |
There was a problem hiding this comment.
Just remembered a case here. Do we privatize the j loop index when converting the do concurrent to parallel + omp do?
do concurrent(i=1:n)
do j=1,m
a(i,j) = i * j
end do
end do
``
There was a problem hiding this comment.
No, it will be shared. This is described in the next paragraph. At least this how the downstream implementation currently works. We can of course descide something when I upstream the next part.
There was a problem hiding this comment.
I would read p 201 of ISO/IEC 1539:1:2023 that j should be privatized unless it has the SAVE attribute. I will send a note to J3/WG5 and make sure that I'm following the right logical path when reading the Fortran spec.
There was a problem hiding this comment.
We can re-open the discussion when the relevant part is upstreamed. This part of the doc was removed until later in any case.
An alternative would be to submit the document as a separate design patch. And then in each subsequent patch update the status of the implementation. |
00f744e to
3d0fc6e
Compare
|
@skatrak @kiranchandramohan I removed the "current status" part of the document. Left the other sections since they are not related to upcoming upstreaming PRs. |
Comment on lines
+7
to
+10
| ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fdo-concurrent-to-openmp=host %s -o - \ | ||
| ! RUN: | FileCheck %s | ||
| ! RUN: bbc -emit-hlfir -fopenmp -fdo-concurrent-to-openmp=host %s -o - \ | ||
| ! RUN: | FileCheck %s |
There was a problem hiding this comment.
Transforms should ideally be tested using the fir-opt tool.
There was a problem hiding this comment.
I will add some MLIR to MLIR tests using fir-opt in later PRs where I will use fir-opt.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ergawy
added a commit
that referenced
this pull request
Mar 10, 2025
Upstreams the next part of `do concurrent` to OpenMP mapping pass (from AMD's ROCm implementation). See #126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range `do concurrent` loops and then map them as "collapsed" loop nests to OpenMP.
d372dd7 to
476819d
Compare
ergawy
added a commit
that referenced
this pull request
Mar 17, 2025
Upstreams the next part of `do concurrent` to OpenMP mapping pass (from AMD's ROCm implementation). See #126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range `do concurrent` loops and then map them as "collapsed" loop nests to OpenMP.
This PR starts the effort to upstream AMD's internal implementation of `do concurrent` to OpenMP mapping. This replaces llvm#77285 since we extended this WIP quite a bit on our fork over the past year. An important part of this PR is a document that describes the current status downstream, the upstreaming status, and next steps to make this pass much more useful. In addition to this document, this PR also contains the skeleton of the pass (no useful transformations are done yet) and some testing for the added command line options.
ergawy
added a commit
that referenced
this pull request
Mar 21, 2025
Upstreams the next part of `do concurrent` to OpenMP mapping pass (from AMD's ROCm implementation). See #126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range `do concurrent` loops and then map them as "collapsed" loop nests to OpenMP.
476819d to
0ee8a8f
Compare
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 27, 2025
…126026) This PR starts the effort to upstream AMD's internal implementation of `do concurrent` to OpenMP mapping. This replaces llvm#77285 since we extended this WIP quite a bit on our fork over the past year. An important part of this PR is a document that describes the current status downstream, the upstreaming status, and next steps to make this pass much more useful. In addition to this document, this PR also contains the skeleton of the pass (no useful transformations are done yet) and some testing for the added command line options. This looks like a huge PR but a lot of the added stuff is documentation. It is also worth noting that the downstream pass has been validated on https://github.com/BerkeleyLab/fiats. For the CPU mapping, this achived performance speed-ups that match pure OpenMP, for GPU mapping we are still working on extending our support for implicit memory mapping and locality specifiers. PR stack: - llvm#126026 (this PR) - llvm#127595 - llvm#127633 - llvm#127634 - llvm#127635
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 27, 2025
…27595) Upstreams the next part of do concurrent to OpenMP mapping pass (from AMD's ROCm implementation). See llvm#126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range do concurrent loops and then map them as "collapsed" loop nests to OpenMP. This is a follow up for llvm#126026, only the latest commit is relevant. This is a replacement for llvm#127478 using a `/user/<username>/<branchname>` branch. PR stack: - llvm#126026 - llvm#127595 (this PR) - llvm#127633 - llvm#127634 - llvm#127635
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 27, 2025
…ructs (llvm#127633) Upstreams one more part of the ROCm `do concurrent` to OpenMP mapping pass. This PR add support for converting simple loops to the equivalent OpenMP constructs on the host: `omp parallel do`. Towards that end, we have to collect more information about loop nests for which we add new utils in the `looputils` name space. PR stack: - llvm#126026 - llvm#127595 - llvm#127633 (this PR) - llvm#127634 - llvm#127635
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 27, 2025
…lvm#127634) Adds support for converting mulit-range loops to OpenMP (on the host only for now). The changes here "prepare" a loop nest for collapsing by sinking iteration variables to the innermost `fir.do_loop` op in the nest. PR stack: - llvm#126026 - llvm#127595 - llvm#127633 - llvm#127634 (this PR) - llvm#127635
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 27, 2025
…lvm#127635) Extends `do concurrent` mapping to handle "loop-local values". A loop-local value is one that is used exclusively inside the loop but allocated outside of it. This usually corresponds to temporary values that are used inside the loop body for initialzing other variables for example. After collecting these values, the pass localizes them to the loop nest by moving their allocations. PR stack: - llvm#126026 - llvm#127595 - llvm#127633 - llvm#127634 - llvm#127635 (this PR)
ergawy
added a commit
that referenced
this pull request
Apr 2, 2025
Upstreams the next part of `do concurrent` to OpenMP mapping pass (from AMD's ROCm implementation). See #126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range `do concurrent` loops and then map them as "collapsed" loop nests to OpenMP.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 2, 2025
…ping (#126026) This PR starts the effort to upstream AMD's internal implementation of `do concurrent` to OpenMP mapping. This replaces #77285 since we extended this WIP quite a bit on our fork over the past year. An important part of this PR is a document that describes the current status downstream, the upstreaming status, and next steps to make this pass much more useful. In addition to this document, this PR also contains the skeleton of the pass (no useful transformations are done yet) and some testing for the added command line options. This looks like a huge PR but a lot of the added stuff is documentation. It is also worth noting that the downstream pass has been validated on https://github.com/BerkeleyLab/fiats. For the CPU mapping, this achived performance speed-ups that match pure OpenMP, for GPU mapping we are still working on extending our support for implicit memory mapping and locality specifiers. PR stack: - llvm/llvm-project#126026 (this PR) - llvm/llvm-project#127595 - llvm/llvm-project#127633 - llvm/llvm-project#127634 - llvm/llvm-project#127635
ergawy
added a commit
that referenced
this pull request
Apr 2, 2025
Upstreams the next part of do concurrent to OpenMP mapping pass (from AMD's ROCm implementation). See #126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range do concurrent loops and then map them as "collapsed" loop nests to OpenMP. This is a follow up for #126026, only the latest commit is relevant. This is a replacement for #127478 using a `/user/<username>/<branchname>` branch. PR stack: - #126026 - #127595 (this PR) - #127633 - #127634 - #127635
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 2, 2025
…on. (#127595) Upstreams the next part of do concurrent to OpenMP mapping pass (from AMD's ROCm implementation). See llvm/llvm-project#126026 for more context. This PR add loop nest detection logic. This enables us to discover muli-range do concurrent loops and then map them as "collapsed" loop nests to OpenMP. This is a follow up for llvm/llvm-project#126026, only the latest commit is relevant. This is a replacement for llvm/llvm-project#127478 using a `/user/<username>/<branchname>` branch. PR stack: - llvm/llvm-project#126026 - llvm/llvm-project#127595 (this PR) - llvm/llvm-project#127633 - llvm/llvm-project#127634 - llvm/llvm-project#127635
ergawy
added a commit
that referenced
this pull request
Apr 2, 2025
…ructs (#127633) Upstreams one more part of the ROCm `do concurrent` to OpenMP mapping pass. This PR add support for converting simple loops to the equivalent OpenMP constructs on the host: `omp parallel do`. Towards that end, we have to collect more information about loop nests for which we add new utils in the `looputils` name space. PR stack: - #126026 - #127595 - #127633 (this PR) - #127634 - #127635
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 2, 2025
… host constructs (#127633) Upstreams one more part of the ROCm `do concurrent` to OpenMP mapping pass. This PR add support for converting simple loops to the equivalent OpenMP constructs on the host: `omp parallel do`. Towards that end, we have to collect more information about loop nests for which we add new utils in the `looputils` name space. PR stack: - llvm/llvm-project#126026 - llvm/llvm-project#127595 - llvm/llvm-project#127633 (this PR) - llvm/llvm-project#127634 - llvm/llvm-project#127635
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/7860 Here is the relevant piece of the build log for the reference |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 2, 2025
…nge loops (#127634) Adds support for converting mulit-range loops to OpenMP (on the host only for now). The changes here "prepare" a loop nest for collapsing by sinking iteration variables to the innermost `fir.do_loop` op in the nest. PR stack: - llvm/llvm-project#126026 - llvm/llvm-project#127595 - llvm/llvm-project#127633 - llvm/llvm-project#127634 (this PR) - llvm/llvm-project#127635
ergawy
added a commit
that referenced
this pull request
Apr 2, 2025
…127635) Extends `do concurrent` mapping to handle "loop-local values". A loop-local value is one that is used exclusively inside the loop but allocated outside of it. This usually corresponds to temporary values that are used inside the loop body for initialzing other variables for example. After collecting these values, the pass localizes them to the loop nest by moving their allocations. PR stack: - #126026 - #127595 - #127633 - #127634 - #127635 (this PR)
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Apr 2, 2025
…nt` nests (#127635) Extends `do concurrent` mapping to handle "loop-local values". A loop-local value is one that is used exclusively inside the loop but allocated outside of it. This usually corresponds to temporary values that are used inside the loop body for initialzing other variables for example. After collecting these values, the pass localizes them to the loop nest by moving their allocations. PR stack: - llvm/llvm-project#126026 - llvm/llvm-project#127595 - llvm/llvm-project#127633 - llvm/llvm-project#127634 - llvm/llvm-project#127635 (this PR)
|
@ergawy Nice work! Thanks! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/201/builds/3924 Here is the relevant piece of the build log for the reference |
I we can just disable this test for powerpc. Opened a PR here: #134249. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR starts the effort to upstream AMD's internal implementation of
do concurrentto OpenMP mapping. This replaces #77285 since we extended this WIP quite a bit on our fork over the past year.An important part of this PR is a document that describes the current status downstream, the upstreaming status, and next steps to make this pass much more useful.
In addition to this document, this PR also contains the skeleton of the pass (no useful transformations are done yet) and some testing for the added command line options.
This looks like a huge PR but a lot of the added stuff is documentation.
It is also worth noting that the downstream pass has been validated on https://github.com/BerkeleyLab/fiats. For the CPU mapping, this achived performance speed-ups that match pure OpenMP, for GPU mapping we are still working on extending our support for implicit memory mapping and locality specifiers.
PR stack:
do concurrentmapping #126026 (this PR)do concurrentloop-nest detection. #127595do concurrentloops to OpenMP host constructs #127633do concurrentmapping to multi-range loops #127634do concurrentnests #127635