Skip to content

Commit 4b15c0e

Browse files
skatrakagozillon
skatrak
and
agozillon
authored
[Flang][HLFIR][OpenMP] Fix offloading tests broken by HLFIR (llvm#69457)
This patch makes changes to the early outlining pass to avoid compiler crashes due to not handling `hlfir.declare` operations correctly. That pass is intended to eventually be removed (llvm#67319), but in the meantime this fixes some issues arising in different parts of the OpenMP offloading compilation process. The main changes included in this patch are the following: - Added support for mapped values defined by an `hlfir.declare` operation. These operations are now kept in outlined target functions, so that both of their outputs (base and original base) are available to the corresponding `omp.target`'s map arguments and region. - Added a fix by @agozillon to prevent unused map clauses from producing a compiler crash. All these unused mapped variables are added to the outlined function's inputs. - Added a fix to the OpenMP translation to MLIR to support integer arguments to these outlined functions. This enables successfully compiling and running the tests in opemp/libomptarget/test/offloading/fortran using HLFIR. Co-authored-by: agozillon <[email protected]>
1 parent 2326b2b commit 4b15c0e

File tree

6 files changed

+66
-24
lines changed

6 files changed

+66
-24
lines changed

‎flang/lib/Optimizer/Transforms/OMPEarlyOutlining.cpp

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "flang/Optimizer/Dialect/FIRDialect.h"
22
#include "flang/Optimizer/Dialect/FIROps.h"
33
#include "flang/Optimizer/Dialect/FIRType.h"
4+
#include "flang/Optimizer/HLFIR/HLFIROps.h"
45
#include "flang/Optimizer/Support/InternalNames.h"
56
#include "flang/Optimizer/Transforms/Passes.h"
67
#include "mlir/Dialect/Func/IR/FuncOps.h"
@@ -99,6 +100,20 @@ class OMPEarlyOutliningPass
99100
return;
100101
}
101102

103+
// Clone into the outlined function all hlfir.declare ops that define inputs
104+
// to the target region and set up remapping of its inputs and outputs.
105+
if (auto declareOp = mlir::dyn_cast_if_present<hlfir::DeclareOp>(
106+
varPtr.getDefiningOp())) {
107+
auto clone = llvm::cast<hlfir::DeclareOp>(
108+
cloneArgAndChildren(builder, declareOp, inputs, newInputs));
109+
mlir::Value newBase = clone.getBase();
110+
mlir::Value newOrigBase = clone.getOriginalBase();
111+
mapInfoMap.map(varPtr, newOrigBase);
112+
valueMap.map(declareOp.getBase(), newBase);
113+
valueMap.map(declareOp.getOriginalBase(), newOrigBase);
114+
return;
115+
}
116+
102117
if (isAddressOfGlobalDeclareTarget(varPtr)) {
103118
fir::AddrOfOp addrOp =
104119
mlir::dyn_cast<fir::AddrOfOp>(varPtr.getDefiningOp());
@@ -127,19 +142,46 @@ class OMPEarlyOutliningPass
127142
llvm::SetVector<mlir::Value> inputs;
128143
mlir::Region &targetRegion = targetOp.getRegion();
129144
mlir::getUsedValuesDefinedAbove(targetRegion, inputs);
130-
131-
// filter out declareTarget and map entries which are specially handled
145+
146+
// Collect all map info. Even non-used maps must be collected to avoid ICEs.
147+
for (mlir::Value oper : targetOp->getOperands()) {
148+
if (auto mapEntry =
149+
mlir::dyn_cast<mlir::omp::MapInfoOp>(oper.getDefiningOp())) {
150+
if (!inputs.contains(mapEntry.getVarPtr()))
151+
inputs.insert(mapEntry.getVarPtr());
152+
}
153+
}
154+
155+
// Filter out declare-target and map entries which are specially handled
132156
// at the moment, so we do not wish these to end up as function arguments
133157
// which would just be more noise in the IR.
158+
llvm::SmallVector<mlir::Value> blockArgs;
134159
for (llvm::SetVector<mlir::Value>::iterator iter = inputs.begin(); iter != inputs.end();) {
135160
if (mlir::isa_and_nonnull<mlir::omp::MapInfoOp>(iter->getDefiningOp()) ||
136161
isAddressOfGlobalDeclareTarget(*iter)) {
137162
iter = inputs.erase(iter);
163+
} else if (auto declareOp = mlir::dyn_cast_if_present<hlfir::DeclareOp>(
164+
iter->getDefiningOp())) {
165+
// Gather hlfir.declare arguments to be added later, after the
166+
// hlfir.declare operation itself has been removed as an input.
167+
blockArgs.push_back(declareOp.getMemref());
168+
if (mlir::Value shape = declareOp.getShape())
169+
blockArgs.push_back(shape);
170+
for (mlir::Value typeParam : declareOp.getTypeparams())
171+
blockArgs.push_back(typeParam);
172+
iter = inputs.erase(iter);
138173
} else {
139174
++iter;
140175
}
141176
}
142177

178+
// Add function arguments to the list of inputs if they are used by an
179+
// hlfir.declare operation.
180+
for (mlir::Value arg : blockArgs) {
181+
if (!arg.getDefiningOp() && !inputs.contains(arg))
182+
inputs.insert(arg);
183+
}
184+
143185
// Create new function and initialize
144186
mlir::FunctionType funcType = builder.getFunctionType(
145187
mlir::TypeRange(inputs.getArrayRef()), mlir::TypeRange());
@@ -218,7 +260,7 @@ class OMPEarlyOutliningPass
218260
return newFunc;
219261
}
220262

221-
// Returns true if a target region was found int the function.
263+
// Returns true if a target region was found in the function.
222264
bool outlineTargetOps(mlir::OpBuilder &builder,
223265
mlir::func::FuncOp &functionOp,
224266
mlir::ModuleOp &moduleOp,

‎flang/test/Lower/OpenMP/FIR/array-bounds.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ end subroutine read_write_section
4848
module assumed_array_routines
4949
contains
5050
!DEVICE-LABEL: func.func @_QMassumed_array_routinesPassumed_shape_array_omp_outline_0(
51-
!DEVICE-SAME: %[[ARG0:.*]]: !fir.ref<i32>, %[[ARG1:.*]]: !fir.box<!fir.array<?xi32>>) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>, omp.outline_parent_name = "_QMassumed_array_routinesPassumed_shape_array"} {
51+
!DEVICE-SAME: %[[ARG0:.*]]: !fir.ref<i32>, %[[ARG1:.*]]: !fir.box<!fir.array<?xi32>>, %[[ARG2:.*]]: !fir.ref<!fir.array<?xi32>>) attributes {omp.declare_target = #omp.declaretarget<device_type = (host), capture_clause = (to)>, omp.outline_parent_name = "_QMassumed_array_routinesPassumed_shape_array"} {
5252
!DEVICE: %[[C0:.*]] = arith.constant 1 : index
5353
!DEVICE: %[[C1:.*]] = arith.constant 4 : index
5454
!DEVICE: %[[C2:.*]] = arith.constant 0 : index

‎flang/test/Lower/OpenMP/FIR/declare-target-implicit-tarop-cap.f90 renamed to ‎flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
2-
!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
3-
!RUN: bbc -emit-fir -fopenmp %s -o - | FileCheck %s
4-
!RUN: bbc -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s --check-prefix=DEVICE
1+
!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
2+
!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
3+
!RUN: bbc -emit-hlfir -fopenmp %s -o - | FileCheck %s
4+
!RUN: bbc -emit-hlfir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s --check-prefix=DEVICE
55

66
! DEVICE-LABEL: func.func @_QPimplicit_capture
77
! DEVICE-SAME: {{.*}}attributes {omp.declare_target = #omp.declaretarget<device_type = (nohost), capture_clause = (to)>{{.*}}}

‎flang/test/Lower/OpenMP/FIR/function-filtering-2.f90 renamed to ‎flang/test/Lower/OpenMP/function-filtering-2.f90

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck --check-prefix=LLVM %s
2-
! RUN: %flang_fc1 -fopenmp -emit-mlir %s -o - | FileCheck --check-prefix=MLIR %s
3-
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-llvm %s -o - | FileCheck --check-prefix=LLVM %s
4-
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-mlir %s -o - | FileCheck --check-prefix=MLIR %s
5-
! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
6-
! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
1+
! RUN: %flang_fc1 -fopenmp -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefix=LLVM %s
2+
! RUN: %flang_fc1 -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefix=MLIR %s
3+
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefix=LLVM %s
4+
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefix=MLIR %s
5+
! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
6+
! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
77

88
! MLIR: func.func @{{.*}}implicit_invocation() attributes {omp.declare_target = #omp.declaretarget<device_type = (nohost), capture_clause = (to)>}
99
! MLIR: return

‎flang/test/Lower/OpenMP/FIR/function-filtering.f90 renamed to ‎flang/test/Lower/OpenMP/function-filtering.f90

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-HOST,LLVM-ALL %s
2-
! RUN: %flang_fc1 -fopenmp -emit-mlir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
3-
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-DEVICE,LLVM-ALL %s
4-
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-mlir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
5-
! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
6-
! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
1+
! RUN: %flang_fc1 -fopenmp -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-HOST,LLVM-ALL %s
2+
! RUN: %flang_fc1 -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
3+
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -flang-experimental-hlfir -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-DEVICE,LLVM-ALL %s
4+
! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
5+
! RUN: bbc -fopenmp -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
6+
! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-hlfir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
77

88
! Check that the correct LLVM IR functions are kept for the host and device
99
! after running the whole set of translation and transformation passes from

‎mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,13 +2046,13 @@ createDeviceArgumentAccessor(llvm::Argument &arg, llvm::Value *input,
20462046
: llvm::Type::getInt64Ty(builder.getContext()),
20472047
ompBuilder.M.getDataLayout().getAllocaAddrSpace());
20482048
llvm::Value *addrAscast =
2049-
builder.CreatePointerBitCastOrAddrSpaceCast(addr, input->getType());
2050-
builder.CreateStore(&arg, addrAscast);
2049+
arg.getType()->isPointerTy()
2050+
? builder.CreatePointerBitCastOrAddrSpaceCast(addr, input->getType())
2051+
: addr;
20512052

2053+
builder.CreateStore(&arg, addrAscast);
20522054
builder.restoreIP(codeGenIP);
2053-
20542055
retVal = builder.CreateLoad(arg.getType(), addrAscast);
2055-
20562056
return builder.saveIP();
20572057
}
20582058

0 commit comments

Comments
 (0)