Skip to content

Commit e9798df

Browse files
committed
[Flang][OpenMP] Upstream -ve bounds fix tweaks that fix regression
Found a regression I made with this PR during upstreaming, this patch should re-align downstream with the upstream PR. Important to land it downstream just now as it fixes a fairly easy to trip regression.
1 parent e298b44 commit e9798df

File tree

1 file changed

+23
-21
lines changed

1 file changed

+23
-21
lines changed

flang/lib/Lower/OpenMP/Utils.cpp

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <flang/Lower/OpenMP/Clauses.h>
2121
#include <flang/Lower/PFTBuilder.h>
2222
#include <flang/Lower/StatementContext.h>
23+
#include <flang/Lower/Support/PrivateReductionUtils.h>
2324
#include <flang/Lower/SymbolMap.h>
2425
#include <flang/Optimizer/Builder/FIRBuilder.h>
2526
#include <flang/Optimizer/Builder/Todo.h>
@@ -301,27 +302,17 @@ mlir::Value createParentSymAndGenIntermediateMaps(
301302
assert(!subscriptIndices.empty() &&
302303
"missing expected indices for map clause");
303304
if (auto boxTy = llvm::dyn_cast<fir::BaseBoxType>(curValue.getType())) {
304-
fir::ExtendedValue exv =
305-
hlfir::translateToExtendedValue(clauseLocation, firOpBuilder,
306-
hlfir::Entity{curValue},
307-
/*contiguousHint=*/
308-
true)
309-
.first;
310-
311-
mlir::Type idxTy = firOpBuilder.getIndexType();
312-
llvm::SmallVector<mlir::Value> shiftOperands;
313-
for (unsigned dim = 0; dim < exv.rank(); ++dim) {
314-
mlir::Value d =
315-
firOpBuilder.createIntegerConstant(clauseLocation, idxTy, dim);
316-
auto dimInfo = fir::BoxDimsOp::create(
317-
firOpBuilder, clauseLocation, idxTy, idxTy, idxTy, curValue, d);
318-
shiftOperands.push_back(dimInfo.getLowerBound());
319-
shiftOperands.push_back(dimInfo.getExtent());
320-
}
321-
auto shapeShiftType =
322-
fir::ShapeShiftType::get(firOpBuilder.getContext(), exv.rank());
323-
mlir::Value shapeShift = fir::ShapeShiftOp::create(
324-
firOpBuilder, clauseLocation, shapeShiftType, shiftOperands);
305+
// To accommodate indexing into box types of all dimensions including
306+
// negative dimensions we have to take into consideration the lower
307+
// bounds and extents of the data (stored in the box) and convey it
308+
// to the ArrayCoorOp so that it can appropriately access the element
309+
// utilising the subscript we provide and the runtime sizes stored in
310+
// the Box. To do so we need to generate a ShapeShiftOp which combines
311+
// both the lb (ShiftOp) and extent (ShapeOp) of the Box, giving the
312+
// ArrayCoorOp the spatial information it needs to calculate the
313+
// underlying address.
314+
mlir::Value shapeShift = Fortran::lower::getShapeShift(
315+
firOpBuilder, clauseLocation, curValue);
325316
auto addrOp =
326317
fir::BoxAddrOp::create(firOpBuilder, clauseLocation, curValue);
327318
curValue = fir::ArrayCoorOp::create(
@@ -330,6 +321,17 @@ mlir::Value createParentSymAndGenIntermediateMaps(
330321
/*slice=*/mlir::Value{}, subscriptIndices,
331322
/*typeparms=*/mlir::ValueRange{});
332323
} else {
324+
// We're required to negate by one in the non-Box case as I believe
325+
// we do not have the shape generated from the dimensions to help
326+
// adjust the indexing.
327+
// TODO/FIXME: This may need adjusted to support bounds of unusual
328+
// dimensions, if that's the case then it is likely best to fold this
329+
// branch into the above.
330+
mlir::Value one = firOpBuilder.createIntegerConstant(
331+
clauseLocation, firOpBuilder.getIndexType(), 1);
332+
for (auto &v : subscriptIndices)
333+
v = mlir::arith::SubIOp::create(firOpBuilder, clauseLocation, v,
334+
one);
333335
curValue = fir::CoordinateOp::create(
334336
firOpBuilder, clauseLocation,
335337
firOpBuilder.getRefType(arrType.getEleTy()), curValue,

0 commit comments

Comments
 (0)