Skip to content

Commit f62de23

Browse files
kparzyszkparzysz
committed
Address review comments
1 parent e2272cb commit f62de23

File tree

5 files changed

+22
-36
lines changed

5 files changed

+22
-36
lines changed

‎flang/include/flang/Support/OpenMP-utils.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ struct EntryBlockArgsEntry {
3434
/// Structure holding the information needed to create and bind entry block
3535
/// arguments associated to all clauses that can define them.
3636
struct EntryBlockArgs {
37-
llvm::ArrayRef<mlir::Value> hostEvalVars;
3837
EntryBlockArgsEntry hasDeviceAddr;
38+
llvm::ArrayRef<mlir::Value> hostEvalVars;
3939
EntryBlockArgsEntry inReduction;
4040
EntryBlockArgsEntry map;
4141
EntryBlockArgsEntry priv;

‎flang/lib/Lower/OpenMP/OpenMP.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,19 +2291,19 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
22912291

22922292
auto targetOp = firOpBuilder.create<mlir::omp::TargetOp>(loc, clauseOps);
22932293

2294-
llvm::SmallVector<mlir::Value> mapBaseValues, hasDeviceAddrBaseValues;
2295-
extractMappedBaseValues(clauseOps.mapVars, mapBaseValues);
2294+
llvm::SmallVector<mlir::Value> hasDeviceAddrBaseValues, mapBaseValues;
22962295
extractMappedBaseValues(clauseOps.hasDeviceAddrVars, hasDeviceAddrBaseValues);
2296+
extractMappedBaseValues(clauseOps.mapVars, mapBaseValues);
22972297

22982298
EntryBlockArgs args;
2299+
args.hasDeviceAddr.syms = hasDeviceAddrSyms;
2300+
args.hasDeviceAddr.vars = hasDeviceAddrBaseValues;
22992301
args.hostEvalVars = clauseOps.hostEvalVars;
23002302
// TODO: Add in_reduction syms and vars.
23012303
args.map.syms = mapSyms;
23022304
args.map.vars = mapBaseValues;
23032305
args.priv.syms = dsp.getDelayedPrivSymbols();
23042306
args.priv.vars = clauseOps.privateVars;
2305-
args.hasDeviceAddr.syms = hasDeviceAddrSyms;
2306-
args.hasDeviceAddr.vars = hasDeviceAddrBaseValues;
23072307

23082308
genBodyOfTargetOp(converter, symTable, semaCtx, eval, targetOp, args, loc,
23092309
queue, item, dsp);

‎flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ class MapInfoFinalizationPass
268268
/// Check if the mapOp is present in the HasDeviceAddr clause on
269269
/// the userOp. Only applies to TargetOp.
270270
bool isHasDeviceAddr(mlir::omp::MapInfoOp mapOp, mlir::Operation *userOp) {
271+
assert(userOp && "Expecting non-null argument");
271272
if (auto targetOp = llvm::dyn_cast<mlir::omp::TargetOp>(userOp)) {
272273
for (mlir::Value hda : targetOp.getHasDeviceAddrVars()) {
273274
if (hda.getDefiningOp() == mapOp)

‎mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1326,7 +1326,7 @@ def TargetOp : OpenMP_Op<"target", traits = [
13261326
}] # clausesExtraClassDeclaration;
13271327

13281328
let assemblyFormat = clausesAssemblyFormat # [{
1329-
custom<HasDeviceAddrHostEvalInReductionMapPrivateRegion>(
1329+
custom<TargetOpRegion>(
13301330
$region, $has_device_addr_vars, type($has_device_addr_vars),
13311331
$host_eval_vars, type($host_eval_vars), $in_reduction_vars,
13321332
type($in_reduction_vars), $in_reduction_byref, $in_reduction_syms,

‎mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp‎

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -808,9 +808,9 @@ static ParseResult parseBlockArgRegion(OpAsmParser &parser, Region &region,
808808
return parser.parseRegion(region, entryBlockArgs);
809809
}
810810

811-
// See custom<HasDeviceAddrHostEvalInReductionMapPrivateRegion> in the
812-
// definition of TargetOp.
813-
static ParseResult parseHasDeviceAddrHostEvalInReductionMapPrivateRegion(
811+
// These parseXyz functions correspond to the custom<Xyz> definitions
812+
// in the .td file(s).
813+
static ParseResult parseTargetOpRegion(
814814
OpAsmParser &parser, Region &region,
815815
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &hasDeviceAddrVars,
816816
SmallVectorImpl<Type> &hasDeviceAddrTypes,
@@ -835,7 +835,6 @@ static ParseResult parseHasDeviceAddrHostEvalInReductionMapPrivateRegion(
835835
return parseBlockArgRegion(parser, region, args);
836836
}
837837

838-
// See custom<InReductionPrivateRegion> in the definition of TaskOp.
839838
static ParseResult parseInReductionPrivateRegion(
840839
OpAsmParser &parser, Region &region,
841840
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &inReductionVars,
@@ -850,8 +849,6 @@ static ParseResult parseInReductionPrivateRegion(
850849
return parseBlockArgRegion(parser, region, args);
851850
}
852851

853-
// See custom<InReductionPrivateReductionRegion> in the definition of
854-
// TaskloopOp.
855852
static ParseResult parseInReductionPrivateReductionRegion(
856853
OpAsmParser &parser, Region &region,
857854
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &inReductionVars,
@@ -872,7 +869,6 @@ static ParseResult parseInReductionPrivateReductionRegion(
872869
return parseBlockArgRegion(parser, region, args);
873870
}
874871

875-
// See custom<PrivateRegion> in the definition of SingleOp.
876872
static ParseResult parsePrivateRegion(
877873
OpAsmParser &parser, Region &region,
878874
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
@@ -882,7 +878,6 @@ static ParseResult parsePrivateRegion(
882878
return parseBlockArgRegion(parser, region, args);
883879
}
884880

885-
// See custom<PrivateReductionRegion> in the definition of LoopOp.
886881
static ParseResult parsePrivateReductionRegion(
887882
OpAsmParser &parser, Region &region,
888883
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
@@ -898,7 +893,6 @@ static ParseResult parsePrivateReductionRegion(
898893
return parseBlockArgRegion(parser, region, args);
899894
}
900895

901-
// See custom<TaskReductionRegion> in the definition of TaskgroupOp.
902896
static ParseResult parseTaskReductionRegion(
903897
OpAsmParser &parser, Region &region,
904898
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &taskReductionVars,
@@ -910,8 +904,6 @@ static ParseResult parseTaskReductionRegion(
910904
return parseBlockArgRegion(parser, region, args);
911905
}
912906

913-
// See custom<UseDeviceAddrUseDevicePtrRegion> in the definition of
914-
// TargetDataOp.
915907
static ParseResult parseUseDeviceAddrUseDevicePtrRegion(
916908
OpAsmParser &parser, Region &region,
917909
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &useDeviceAddrVars,
@@ -1073,17 +1065,18 @@ static void printBlockArgRegion(OpAsmPrinter &p, Operation *op, Region &region,
10731065
p.printRegion(region, /*printEntryBlockArgs=*/false);
10741066
}
10751067

1076-
// See custom<HasDeviceAddrHostEvalInReductionMapPrivateRegion> in the
1077-
// definition of TargetOp.
1078-
static void printHasDeviceAddrHostEvalInReductionMapPrivateRegion(
1079-
OpAsmPrinter &p, Operation *op, Region &region,
1080-
ValueRange hasDeviceAddrVars, TypeRange hasDeviceAddrTypes,
1081-
ValueRange hostEvalVars, TypeRange hostEvalTypes,
1082-
ValueRange inReductionVars, TypeRange inReductionTypes,
1083-
DenseBoolArrayAttr inReductionByref, ArrayAttr inReductionSyms,
1084-
ValueRange mapVars, TypeRange mapTypes, ValueRange privateVars,
1085-
TypeRange privateTypes, ArrayAttr privateSyms,
1086-
DenseI64ArrayAttr privateMaps) {
1068+
// These parseXyz functions correspond to the custom<Xyz> definitions
1069+
// in the .td file(s).
1070+
static void
1071+
printTargetOpRegion(OpAsmPrinter &p, Operation *op, Region &region,
1072+
ValueRange hasDeviceAddrVars, TypeRange hasDeviceAddrTypes,
1073+
ValueRange hostEvalVars, TypeRange hostEvalTypes,
1074+
ValueRange inReductionVars, TypeRange inReductionTypes,
1075+
DenseBoolArrayAttr inReductionByref,
1076+
ArrayAttr inReductionSyms, ValueRange mapVars,
1077+
TypeRange mapTypes, ValueRange privateVars,
1078+
TypeRange privateTypes, ArrayAttr privateSyms,
1079+
DenseI64ArrayAttr privateMaps) {
10871080
AllRegionPrintArgs args;
10881081
args.hasDeviceAddrArgs.emplace(hasDeviceAddrVars, hasDeviceAddrTypes);
10891082
args.hostEvalArgs.emplace(hostEvalVars, hostEvalTypes);
@@ -1094,7 +1087,6 @@ static void printHasDeviceAddrHostEvalInReductionMapPrivateRegion(
10941087
printBlockArgRegion(p, op, region, args);
10951088
}
10961089

1097-
// See custom<InReductionPrivateRegion> in the definition of TaskOp.
10981090
static void printInReductionPrivateRegion(
10991091
OpAsmPrinter &p, Operation *op, Region &region, ValueRange inReductionVars,
11001092
TypeRange inReductionTypes, DenseBoolArrayAttr inReductionByref,
@@ -1108,8 +1100,6 @@ static void printInReductionPrivateRegion(
11081100
printBlockArgRegion(p, op, region, args);
11091101
}
11101102

1111-
// See custom<InReductionPrivateReductionRegion> in the definition of
1112-
// TaskloopOp.
11131103
static void printInReductionPrivateReductionRegion(
11141104
OpAsmPrinter &p, Operation *op, Region &region, ValueRange inReductionVars,
11151105
TypeRange inReductionTypes, DenseBoolArrayAttr inReductionByref,
@@ -1127,7 +1117,6 @@ static void printInReductionPrivateReductionRegion(
11271117
printBlockArgRegion(p, op, region, args);
11281118
}
11291119

1130-
// See custom<PrivateRegion> in the definition of SingleOp.
11311120
static void printPrivateRegion(OpAsmPrinter &p, Operation *op, Region &region,
11321121
ValueRange privateVars, TypeRange privateTypes,
11331122
ArrayAttr privateSyms) {
@@ -1137,7 +1126,6 @@ static void printPrivateRegion(OpAsmPrinter &p, Operation *op, Region &region,
11371126
printBlockArgRegion(p, op, region, args);
11381127
}
11391128

1140-
// See custom<PrivateReductionRegion> in the definition of LoopOp.
11411129
static void printPrivateReductionRegion(
11421130
OpAsmPrinter &p, Operation *op, Region &region, ValueRange privateVars,
11431131
TypeRange privateTypes, ArrayAttr privateSyms,
@@ -1152,7 +1140,6 @@ static void printPrivateReductionRegion(
11521140
printBlockArgRegion(p, op, region, args);
11531141
}
11541142

1155-
// See custom<TaskReductionRegion> in the definition of TaskgroupOp.
11561143
static void printTaskReductionRegion(OpAsmPrinter &p, Operation *op,
11571144
Region &region,
11581145
ValueRange taskReductionVars,
@@ -1165,8 +1152,6 @@ static void printTaskReductionRegion(OpAsmPrinter &p, Operation *op,
11651152
printBlockArgRegion(p, op, region, args);
11661153
}
11671154

1168-
// See custom<UseDeviceAddrUseDevicePtrRegion> in the definition of
1169-
// TargetDataOp.
11701155
static void printUseDeviceAddrUseDevicePtrRegion(OpAsmPrinter &p, Operation *op,
11711156
Region &region,
11721157
ValueRange useDeviceAddrVars,

0 commit comments

Comments
 (0)