[OpenMP][Flang] Enable alias analysis inside omp target region #111670
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
At present, alias analysis does not work for operations inside OMP target regions because the FIR declare operations within OMP target do not offer sufficient information for alias analysis. Consequently, it is necessary to examine the FIR code outside the OMP target region.
DominikAdamski
requested review from
skatrak,
jsjodin,
tblah,
agozillon and
kiranktp
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Dominik Adamski (DominikAdamski) ChangesAt present, alias analysis does not work for operations inside OMP target regions because the FIR declare operations within OMP target do not offer sufficient information for alias analysis. Consequently, it is necessary to examine the FIR code outside the OMP target region. Full diff: https://github.com/llvm/llvm-project/pull/111670.diff 3 Files Affected:
diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index e88da5a8ebae19..c168eee69bf7d0 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -13,6 +13,8 @@
#include "flang/Optimizer/Dialect/FortranVariableInterface.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "mlir/Analysis/AliasAnalysis.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/Dialect/OpenMP/OpenMPInterfaces.h"
#include "mlir/IR/BuiltinOps.h"
#include "mlir/IR/Value.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
@@ -296,6 +298,15 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
defOp = v.getDefiningOp();
return;
}
+ // If load is inside target and it points to mapped item,
+ // continue tracking.
+ Operation *loadMemrefOp = op.getMemref().getDefiningOp();
+ if (llvm::isa<fir::DeclareOp>(loadMemrefOp) &&
+ llvm::isa<omp::TargetOp>(loadMemrefOp->getParentOp())) {
+ v = op.getMemref();
+ defOp = v.getDefiningOp();
+ return;
+ }
// No further tracking for addresses loaded from memory for now.
type = SourceKind::Indirect;
breakFromLoop = true;
@@ -319,6 +330,32 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
breakFromLoop = true;
})
.Case<hlfir::DeclareOp, fir::DeclareOp>([&](auto op) {
+ // If declare operation is inside omp target region,
+ // continue alias analysis outside the target region
+ if (llvm::isa<omp::TargetOp>(op->getParentOp())) {
+ omp::TargetOp targetOp =
+ llvm::cast<omp::TargetOp>(op->getParentOp());
+ auto mapClauseOwner =
+ llvm::dyn_cast<omp::MapClauseOwningOpInterface>(
+ targetOp.getOperation());
+ if (!mapClauseOwner) {
+ defOp = nullptr;
+ breakFromLoop = true;
+ return;
+ }
+ Block *targetEntryBlock = &targetOp->getRegion(0).front();
+ OperandRange mapVarsArr = mapClauseOwner.getMapVars();
+ assert(mapVarsArr.size() == targetEntryBlock->getNumArguments());
+ for (size_t i = 0; i < targetEntryBlock->getNumArguments(); i++) {
+ if (targetEntryBlock->getArgument(i) == op.getMemref()) {
+ omp::MapInfoOp mapInfo =
+ llvm::cast<omp::MapInfoOp>(mapVarsArr[i].getDefiningOp());
+ defOp = mapInfo.getVarPtr().getDefiningOp();
+ v = mapInfo.getVarPtr();
+ return;
+ }
+ }
+ }
auto varIf = llvm::cast<fir::FortranVariableOpInterface>(defOp);
// While going through a declare operation collect
// the variable attributes from it. Right now, some
diff --git a/flang/lib/Optimizer/Analysis/CMakeLists.txt b/flang/lib/Optimizer/Analysis/CMakeLists.txt
index 436d4d3f18969c..c000a9da99f871 100644
--- a/flang/lib/Optimizer/Analysis/CMakeLists.txt
+++ b/flang/lib/Optimizer/Analysis/CMakeLists.txt
@@ -6,6 +6,7 @@ add_flang_library(FIRAnalysis
FIRDialect
HLFIRDialect
MLIRIR
+ MLIROpenMPDialect
LINK_LIBS
FIRBuilder
@@ -14,5 +15,6 @@ add_flang_library(FIRAnalysis
MLIRFuncDialect
MLIRLLVMDialect
MLIRMathTransforms
+ MLIROpenMPDialect
FIRSupport
)
diff --git a/flang/test/Analysis/AliasAnalysis/alias-analysis-omp-target-1.fir b/flang/test/Analysis/AliasAnalysis/alias-analysis-omp-target-1.fir
new file mode 100644
index 00000000000000..38fe799d2618ee
--- /dev/null
+++ b/flang/test/Analysis/AliasAnalysis/alias-analysis-omp-target-1.fir
@@ -0,0 +1,66 @@
+// Use --mlir-disable-threading so that the AA queries are serialized
+// as well as its diagnostic output.
+// RUN: fir-opt %s -pass-pipeline='builtin.module(func.func(test-fir-alias-analysis))' -split-input-file --mlir-disable-threading 2>&1 | FileCheck %s
+
+// Fortran source code:
+//
+// program TestAllocatableArray
+// real(kind=8), allocatable :: A(:)
+// real(kind=8), allocatable :: B(:)
+// !$omp target
+// A(0) = B(0)
+// !$omp end target
+// end TestAllocatableArray
+
+// CHECK-LABEL: Testing : "_QPTestAllocatableArray"
+// CHECK-DAG: ArrayA#0 <-> ArrayB#0: NoAlias
+func.func @_QPTestAllocatableArray() {
+ %0 = fir.address_of(@_QFEa) : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %1:2 = hlfir.declare %0 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "ArrayA", test.ptr = "ArrayA" } : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>)
+ %2 = fir.address_of(@_QFEb) : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %3:2 = hlfir.declare %2 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "ArrayB", test.ptr = "ArrayB" } : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>)
+ %4 = fir.load %1#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c1 = arith.constant 1 : index
+ %c0 = arith.constant 0 : index
+ %5 = fir.load %1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c0_0 = arith.constant 0 : index
+ %6:3 = fir.box_dims %5, %c0_0 : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> (index, index, index)
+ %7:3 = fir.box_dims %4, %c0 : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> (index, index, index)
+ %c0_1 = arith.constant 0 : index
+ %8 = arith.subi %7#1, %c1 : index
+ %9 = omp.map.bounds lower_bound(%c0_1 : index) upper_bound(%8 : index) extent(%7#1 : index) stride(%7#2 : index) start_idx(%6#0 : index) {stride_in_bytes = true}
+ %10 = fir.box_offset %1#1 base_addr : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>
+ %11 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) var_ptr_ptr(%10 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) bounds(%9) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
+ %12 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.box<!fir.heap<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) members(%11 : [0] : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>> {name = "a"}
+ %13 = fir.load %3#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c1_2 = arith.constant 1 : index
+ %c0_3 = arith.constant 0 : index
+ %14 = fir.load %3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c0_4 = arith.constant 0 : index
+ %15:3 = fir.box_dims %14, %c0_4 : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> (index, index, index)
+ %16:3 = fir.box_dims %13, %c0_3 : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> (index, index, index)
+ %c0_5 = arith.constant 0 : index
+ %17 = arith.subi %16#1, %c1_2 : index
+ %18 = omp.map.bounds lower_bound(%c0_5 : index) upper_bound(%17 : index) extent(%16#1 : index) stride(%16#2 : index) start_idx(%15#0 : index) {stride_in_bytes = true}
+ %19 = fir.box_offset %3#1 base_addr : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>
+ %20 = omp.map.info var_ptr(%3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) var_ptr_ptr(%19 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) bounds(%18) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
+ %21 = omp.map.info var_ptr(%3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.box<!fir.heap<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) members(%20 : [0] : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>> {name = "b"}
+ omp.target map_entries(%11 -> %arg0, %12 -> %arg1, %20 -> %arg2, %21 -> %arg3 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) {
+ %22:2 = hlfir.declare %arg1 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFEa"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>)
+ %23:2 = hlfir.declare %arg3 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFEb"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>)
+ %24 = fir.load %23#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c0_6 = arith.constant 0 : index
+ %25 = hlfir.designate %24 (%c0_6) : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> !fir.ref<f64>
+ %26 = fir.load %25 : !fir.ref<f64>
+ %27 = fir.load %22#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
+ %c0_7 = arith.constant 0 : index
+ %28 = hlfir.designate %27 (%c0_7) : (!fir.box<!fir.heap<!fir.array<?xf64>>>, index) -> !fir.ref<f64>
+ hlfir.assign %26 to %28 : f64, !fir.ref<f64>
+ omp.terminator
+ }
+ return
+}
+fir.global internal @_QFEa : !fir.box<!fir.heap<!fir.array<?xf64>>> {
+}
+fir.global internal @_QFEb : !fir.box<!fir.heap<!fir.array<?xf64>>> {
+}
|
agozillon
approved these changes
Oct 9, 2024
| auto mapClauseOwner = | ||
| llvm::dyn_cast<omp::MapClauseOwningOpInterface>( | ||
| targetOp.getOperation()); | ||
| if (!mapClauseOwner) { |
There was a problem hiding this comment.
I think at the moment TargetOp is always guaranteed to be a MapClauseOwningOpInterface (unless somethings changed with the definition of it or I am misunderstanding something about interfaces), so we might not require this if check unless we're worried about targetOp.getOperation() returning null! But I'll leave if you wish to keep it or not up to you as having the extra layer of safety incase something in the interface or target operation changes could be nice! :-)
There was a problem hiding this comment.
I modified PR because of #109808
tblah
reviewed
Oct 10, 2024
Comment on lines
335
to
337
| if (llvm::isa<omp::TargetOp>(op->getParentOp())) { | ||
| omp::TargetOp targetOp = | ||
| llvm::cast<omp::TargetOp>(op->getParentOp()); |
There was a problem hiding this comment.
nit: This can be written more efficiently as if (auto targetOp = llvm::dyn_cast<omp::TargetOp>(op->getParentOp())).
There was a problem hiding this comment.
Fixed, thanks
tblah
reviewed
Oct 10, 2024
test.ptr attribute should be attached to operation inside target region.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…111670) At present, alias analysis does not work for operations inside OMP target regions because the FIR declare operations within OMP target do not offer sufficient information for alias analysis. Consequently, it is necessary to examine the FIR code outside the OMP target region.
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.
At present, alias analysis does not work for operations inside OMP target regions because the FIR declare operations within OMP target do not offer sufficient information for alias analysis. Consequently, it is necessary to examine the FIR code outside the OMP target region.