[flang-commits] [flang] [Flang][OpenMP] Fix negative array indexing with allocatable derived type array maps (PR #154193)

[via flang-commits]

================
@@ -331,10 +325,41 @@ mlir::Value createParentSymAndGenIntermediateMaps(
                              subscriptIndices, objectList[i]);
         assert(!subscriptIndices.empty() &&
                "missing expected indices for map clause");
-        curValue = fir::CoordinateOp::create(
-            firOpBuilder, clauseLocation,
-            firOpBuilder.getRefType(arrType.getEleTy()), curValue,
-            subscriptIndices);
+        if (auto boxTy = llvm::dyn_cast<fir::BaseBoxType>(curValue.getType())) {
+          fir::ExtendedValue exv =
+              hlfir::translateToExtendedValue(clauseLocation, firOpBuilder,
+                                              hlfir::Entity{curValue},
+                                              /*contiguousHint=*/
+                                              true)
+                  .first;
+
+          mlir::Type idxTy = firOpBuilder.getIndexType();
+          llvm::SmallVector<mlir::Value> shiftOperands;
+          for (unsigned dim = 0; dim < exv.rank(); ++dim) {
+            mlir::Value d =
+                firOpBuilder.createIntegerConstant(clauseLocation, idxTy, dim);
+            auto dimInfo = fir::BoxDimsOp::create(
+                firOpBuilder, clauseLocation, idxTy, idxTy, idxTy, curValue, d);
+            shiftOperands.push_back(dimInfo.getLowerBound());
+            shiftOperands.push_back(dimInfo.getExtent());
+          }
+          auto shapeShiftType =
+              fir::ShapeShiftType::get(firOpBuilder.getContext(), exv.rank());
+          mlir::Value shapeShift = fir::ShapeShiftOp::create(
+              firOpBuilder, clauseLocation, shapeShiftType, shiftOperands);
+          auto addrOp =
+              fir::BoxAddrOp::create(firOpBuilder, clauseLocation, curValue);
+          curValue = fir::ArrayCoorOp::create(
+              firOpBuilder, clauseLocation,
+              firOpBuilder.getRefType(arrType.getEleTy()), addrOp, shapeShift,
+              /*slice=*/mlir::Value{}, subscriptIndices,
+              /*typeparms=*/mlir::ValueRange{});
+        } else {
+          curValue = fir::CoordinateOp::create(
+              firOpBuilder, clauseLocation,
+              firOpBuilder.getRefType(arrType.getEleTy()), curValue,
+              subscriptIndices);
----------------
agozillon wrote:

It's definitely incorrect in the box case, as it'll result in runtime errors. In the regular CoordinateOp case it doesn't result in any (obvious from the tests I have and we have downstream) issues with or without it, I would be happy to re-add it though in the else section which would likely be the more prudent thing to do until I can work out a test-case that'd show which is flawed. I originally added the subtraction as the bit of IR I was looking at to work out the appropriate operations made use of it, so it *seems* like it should be the correct thing to do.    

https://github.com/llvm/llvm-project/pull/154193