[flang] Fixed substr access in embox/rebox CodeGen.

[vzakhari]

Summary

The code was using the original operand of the operation, while
it should have been using the remapped operands via the adaptor.

[clementval]

LGTM

[jeanPerier]

Thanks!

flang/lib/Optimizer/CodeGen/CodeGen.cpp

mlir::Value populateDescriptor(mlir::Location loc, mlir::ModuleOp mod,
     return descriptor;
   }
 
   // Template used for fir::EmboxOp and fir::cg::XEmboxOp
   template <typename BOX>
   std::tuple<fir::BaseBoxType, mlir::Value, mlir::Value>
   consDescriptorPrefix(BOX box, mlir::Type inputType,
                        mlir::ConversionPatternRewriter &rewriter, unsigned rank,
+                       [[maybe_unused]] mlir::ValueRange substrParams,
                        mlir::ValueRange lenParams, mlir::Value sourceBox = {},
                        mlir::Type sourceBoxType = {}) const {
     auto loc = box.getLoc();
     auto boxTy = box.getType().template dyn_cast<fir::BaseBoxType>();
     bool useInputType = fir::isPolymorphicType(boxTy) &&
                         !fir::isUnlimitedPolymorphicType(inputType);
     llvm::SmallVector<mlir::Value> typeparams = lenParams;
     if constexpr (!std::is_same_v<BOX, fir::EmboxOp>) {
       if (!box.getSubstr().empty() && fir::hasDynamicSize(boxTy.getEleTy()))
-        typeparams.push_back(box.getSubstr()[1]);
+        typeparams.push_back(substrParams[1]);
     }
 
     // Write each of the fields with the appropriate values.
     // When emboxing an element to a polymorphic descriptor, use the
     // input type since the destination descriptor type has not the exact
     // information.
     auto [eleSize, cfiTy] = getSizeAndTypeCode(
         loc, rewriter, useInputType ? inputType : boxTy.getEleTy(), typeparams);
mlir::Value descriptor = populateDescriptor(
         loc, mod, boxTy, inputType, rewriter, rank, eleSize, cfiTy, typeDesc);
 
     return {boxTy, descriptor, eleSize};
   }
 
   std::tuple<fir::BaseBoxType, mlir::Value, mlir::Value>
   consDescriptorPrefix(fir::cg::XReboxOp box, mlir::Value loweredBox,
                        mlir::ConversionPatternRewriter &rewriter, unsigned rank,
+                       mlir::ValueRange substrParams,
                        mlir::ValueRange lenParams,
                        mlir::Value typeDesc = {}) const {
     auto loc = box.getLoc();
     auto boxTy = box.getType().dyn_cast<fir::BaseBoxType>();
     auto inputBoxTy = box.getBox().getType().dyn_cast<fir::BaseBoxType>();
     llvm::SmallVector<mlir::Value> typeparams = lenParams;
     if (!box.getSubstr().empty() && fir::hasDynamicSize(boxTy.getEleTy()))
-      typeparams.push_back(box.getSubstr()[1]);
+      typeparams.push_back(substrParams[1]);
 
     auto [eleSize, cfiTy] =
         getSizeAndTypeCode(loc, rewriter, boxTy.getEleTy(), typeparams);
 
     // Reboxing to a polymorphic entity. eleSize and type code need to
     // be retrieved from the initial box and propagated to the new box.
     // If the initial box has an addendum, the type desc must be propagated as
     // well.
matchAndRewrite(fir::EmboxOp embox, OpAdaptor adaptor,
     mlir::Type sourceBoxType;
     if (embox.getSourceBox()) {
       sourceBox = operands[embox.getSourceBoxOffset()];
       sourceBoxType = embox.getSourceBox().getType();
     }
     assert(!embox.getShape() && "There should be no dims on this embox op");
     auto [boxTy, dest, eleSize] = consDescriptorPrefix(
         embox, fir::unwrapRefType(embox.getMemref().getType()), rewriter,
-        /*rank=*/0, /*lenParams=*/operands.drop_front(1), sourceBox,
-        sourceBoxType);
+        /*rank=*/0, /*substrParams=*/mlir::ValueRange{},
+        adaptor.getTypeparams(), sourceBox, sourceBoxType);
     dest = insertBaseAddress(rewriter, embox.getLoc(), dest, operands[0]);
     if (fir::isDerivedTypeWithLenParams(boxTy)) {
       TODO(embox.getLoc(),
            "fir.embox codegen of derived with length parameters");
       return mlir::failure();
     }
     auto result =
         placeInMemoryIfNotGlobalInit(rewriter, embox.getLoc(), boxTy, dest);
matchAndRewrite(fir::cg::XEmboxOp xbox, OpAdaptor adaptor,
     mlir::Value sourceBox;
     mlir::Type sourceBoxType;
     if (xbox.getSourceBox()) {
       sourceBox = operands[xbox.getSourceBoxOffset()];
       sourceBoxType = xbox.getSourceBox().getType();
     }
     auto [boxTy, dest, eleSize] = consDescriptorPrefix(
         xbox, fir::unwrapRefType(xbox.getMemref().getType()), rewriter,
-        xbox.getOutRank(), operands.drop_front(xbox.lenParamOffset()),
+        xbox.getOutRank(), adaptor.getSubstr(), adaptor.getLenParams(),
         sourceBox, sourceBoxType);
     // Generate the triples in the dims field of the descriptor
     auto i64Ty = mlir::IntegerType::get(xbox.getContext(), 64);
     mlir::Value base = operands[0];
     assert(!xbox.getShape().empty() && "must have a shape");
     unsigned shapeOffset = xbox.shapeOffset();
     bool hasShift = !xbox.getShift().empty();
     unsigned shiftOffset = xbox.shiftOffset();
matchAndRewrite(fir::cg::XReboxOp rebox, OpAdaptor adaptor,
     mlir::Value typeDescAddr;
     if (rebox.getBox().getType().isa<fir::ClassType>() &&
         rebox.getType().isa<fir::ClassType>())
       typeDescAddr = loadTypeDescAddress(loc, rebox.getBox().getType(),
                                          loweredBox, rewriter);
 
     auto [boxTy, dest, eleSize] =
         consDescriptorPrefix(rebox, loweredBox, rewriter, rebox.getOutRank(),
-                             lenParams, typeDescAddr);
+                             adaptor.getSubstr(), lenParams, typeDescAddr);
 
     // Read input extents, strides, and base address
     llvm::SmallVector<mlir::Value> inputExtents;
     llvm::SmallVector<mlir::Value> inputStrides;
     const unsigned inputRank = rebox.getRank();
     for (unsigned dim = 0; dim < inputRank; ++dim) {
       llvm::SmallVector<mlir::Value, 3> dimInfo =
           getDimsFromBox(loc, {idxTy, idxTy, idxTy}, rebox.getBox().getType(),

flang/test/Fir/embox-substring.fir

+// RUN: fir-opt -o - -cg-rewrite --fir-to-llvm-ir %s | FileCheck %s
+// RUN: tco -o - -cg-rewrite --fir-to-llvm-ir %s | FileCheck %s
+
+// CHECK-LABEL: llvm.func @embox_index_substr(
+// CHECK-NOT: NULL_VALUE
+// CHECK-NOT: NULL_TYPE
+func.func @embox_index_substr(%addr : !fir.ref<!fir.array<?x!fir.char<1,2>>>) {
+  %0 = arith.constant 0 : index
+  %1 = fir.shape_shift %0, %0 : (index, index) -> !fir.shapeshift<1>
+  %2 = fir.slice %0, %0, %0 substr %0, %0: (index, index, index, index, index) -> !fir.slice<1>
+  %3 = fir.embox %addr (%1) [%2] : (!fir.ref<!fir.array<?x!fir.char<1,2>>>, !fir.shapeshift<1>, !fir.slice<1>) -> !fir.box<!fir.array<?x!fir.char<1,?>>>
+  return
+}

flang/test/Fir/rebox-susbtring.fir

 // CHECK:         llvm.bitcast %[[VAL_33]] : !llvm.ptr<i8> to !llvm.ptr<i8>
 
 // More offset computation with descriptor strides and triplets that is not character specific ...
 
   %2 = fir.rebox %arg0 [%1] : (!fir.box<!fir.array<?x!fir.type<t{i:i32,c:!fir.char<1,10>}>>>, !fir.slice<1>) -> !fir.box<!fir.array<?x!fir.char<1,?>>>
   fir.call @bar(%2) : (!fir.box<!fir.array<?x!fir.char<1,?>>>) -> ()
   return
 }
+
+// Test that a rebox with `index` substring parameter is converted
+// to legal IR. It used to produce:
+//   %63 = "llvm.mul"(%62, <<NULL VALUE>>) : (i64, <<NULL TYPE>>) -> i64
+// because the substr was not accessed via the adaptor's operands.
+
+// CHECK-LABEL: llvm.func @index_substr(
+// CHECK-NOT: NULL_VALUE
+// CHECK-NOT: NULL_TYPE
+func.func @index_substr(%arg0: !fir.box<!fir.array<?x!fir.char<1,20>>>) {
+  %c7_index = arith.constant 7 : index
+  %c1_i64 = arith.constant 1 : i64
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  %0:3 = fir.box_dims %arg0, %c0 : (!fir.box<!fir.array<?x!fir.char<1,20>>>, index) -> (index, index, index)
+  %1 = fir.slice %c1, %0#1, %c1_i64 substr %c1_i64, %c7_index : (index, index, i64, i64, index) -> !fir.slice<1>
+  %2 = fir.rebox %arg0 [%1] : (!fir.box<!fir.array<?x!fir.char<1,20>>>, !fir.slice<1>) -> !fir.box<!fir.array<?x!fir.char<1,?>>>
+  fir.call @bar(%2) : (!fir.box<!fir.array<?x!fir.char<1,?>>>) -> ()
+  return
+}