[flang] Use box for components with non-default lower bounds

[ashermancinelli]

When designating an array component that has non-default lower bounds the bridge was producing hlfir designates yielding reference types, which did not preserve the bounds information. Then, when creating components, unadjusted indices were used when initializing the structure.

We could look at the declaration to get the shape parameter, but this would not be preserved if the component were passed as a block argument. These results must be boxed, but we also must not lose the contiguity information either. To address contiguity, annotate these boxes with the contiguous attribute during designation.

Note that other designated entities are handled inside the HlfirDesignatorBuilder while component designators are built in HlfirBuilder. I am not sure if this handling should be moved into the designator builder or left in the general builder, so feedback is welcome.

Also, I wouldn't mind finding a test that demonstrates a box-designated component with the contiguous attribute really is determined to be contiguous by any passes down the line checking for that. I don't have a test like that yet.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Asher Mancinelli (ashermancinelli)

Changes

When designating an array component that has non-default lower bounds the bridge was producing hlfir designates yielding reference types, which did not preserve the bounds information. Then, when creating components, unadjusted indices were used when initializing the structure.

We could look at the declaration to get the shape parameter, but this would not be preserved if the component were passed as a block argument. These results must be boxed, but we also must not lose the contiguity information either. To address contiguity, annotate these boxes with the contiguous attribute during designation.

Note that other designated entities are handled inside the HlfirDesignatorBuilder while component designators are built in HlfirBuilder. I am not sure if this handling should be moved into the designator builder or left in the general builder, so feedback is welcome.

---

Full diff: https://github.com/llvm/llvm-project/pull/138994.diff

2 Files Affected:

• (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+19-3)
• (modified) flang/test/Lower/HLFIR/designators-component-ref.f90 (+10)

diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 04b63f92a1fb4..da21d407e927d 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -30,6 +30,7 @@
 #include "flang/Optimizer/Builder/Runtime/Derived.h"
 #include "flang/Optimizer/Builder/Runtime/Pointer.h"
 #include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/Dialect/FIRAttr.h"
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
 #include "mlir/IR/IRMapping.h"
 #include "llvm/ADT/TypeSwitch.h"
@@ -125,6 +126,19 @@ class HlfirDesignatorBuilder {
   hlfir::ElementalAddrOp convertVectorSubscriptedExprToElementalAddr(
       const Fortran::lower::SomeExpr &designatorExpr);
 
+  std::tuple<mlir::Type, fir::FortranVariableFlagsEnum>
+  genComponentDesignatorTypeAndAttributes(
+      const Fortran::semantics::Symbol &componentSym, mlir::Type fieldType,
+      bool isVolatile) {
+    if (mayHaveNonDefaultLowerBounds(componentSym)) {
+      mlir::Type boxType = fir::BoxType::get(fieldType, isVolatile);
+      return std::make_tuple(boxType,
+                             fir::FortranVariableFlagsEnum::contiguous);
+    }
+    auto refType = fir::ReferenceType::get(fieldType, isVolatile);
+    return std::make_tuple(refType, fir::FortranVariableFlagsEnum{});
+  }
+
   mlir::Value genComponentShape(const Fortran::semantics::Symbol &componentSym,
                                 mlir::Type fieldType) {
     // For pointers and allocatable components, the
@@ -1863,8 +1877,9 @@ class HlfirBuilder {
           designatorBuilder.genComponentShape(sym, compType);
       const bool isDesignatorVolatile =
           fir::isa_volatile_type(baseOp.getType());
-      mlir::Type designatorType =
-          builder.getRefType(compType, isDesignatorVolatile);
+      auto [designatorType, extraAttributeFlags] =
+          designatorBuilder.genComponentDesignatorTypeAndAttributes(
+              sym, compType, isDesignatorVolatile);
 
       mlir::Type fieldElemType = hlfir::getFortranElementType(compType);
       llvm::SmallVector<mlir::Value, 1> typeParams;
@@ -1884,7 +1899,8 @@ class HlfirBuilder {
 
       // Convert component symbol attributes to variable attributes.
       fir::FortranVariableFlagsAttr attrs =
-          Fortran::lower::translateSymbolAttributes(builder.getContext(), sym);
+          Fortran::lower::translateSymbolAttributes(builder.getContext(), sym,
+                                                    extraAttributeFlags);
 
       // Get the component designator.
       auto lhs = builder.create<hlfir::DesignateOp>(
diff --git a/flang/test/Lower/HLFIR/designators-component-ref.f90 b/flang/test/Lower/HLFIR/designators-component-ref.f90
index 653e28e0a6018..935176becac75 100644
--- a/flang/test/Lower/HLFIR/designators-component-ref.f90
+++ b/flang/test/Lower/HLFIR/designators-component-ref.f90
@@ -126,6 +126,16 @@ subroutine test_array_comp_non_contiguous_slice(a)
 ! CHECK:  %[[VAL_19:.*]] = hlfir.designate %[[VAL_1]]#0{"array_comp"} <%[[VAL_9]]> (%[[VAL_10]]:%[[VAL_11]]:%[[VAL_12]], %[[VAL_14]]:%[[VAL_15]]:%[[VAL_16]])  shape %[[VAL_18]] : (!fir.ref<!fir.type<_QMcomp_refTt_array{scalar_i:i32,array_comp:!fir.array<10x20xf32>}>>, !fir.shape<2>, index, index, index, index, index, index, !fir.shape<2>) -> !fir.box<!fir.array<6x17xf32>>
 end subroutine
 
+subroutine test_array_lbs_array_ctor()
+  use comp_ref
+  type(t_array_lbs) :: a(-1:1)
+  real :: array_comp(2:11,3:22)
+  a = (/ (t_array_lbs(i, array_comp), i=-1,1) /)
+! CHECK: hlfir.designate %{{.+}}#0{"array_comp_lbs"} <%{{.+}}> shape %{{.+}} {fortran_attrs = #fir.var_attrs<contiguous>}
+! CHECK-SAME: (!fir.ref<!fir.type<_QMcomp_refTt_array_lbs{scalar_i:i32,array_comp_lbs:!fir.array<10x20xf32>}>>, !fir.shapeshift<2>, !fir.shapeshift<2>)
+! CHECK-SAME: -> !fir.box<!fir.array<10x20xf32>>
+end subroutine
+
 subroutine test_array_lbs_comp_lbs_1(a)
   use comp_ref
   type(t_array_lbs) :: a

[vzakhari]

LGTM. Thank you, Asher.

OptimizedBufferization is one pass that cares about the continuity. E.g.:

subroutine test(x)
  interface
    function array_func
      real :: array_func(10)
    end
  end interface
  real :: x(10)
  x = array_func()
end

The array_func call is done within hlfir.eval_in_mem, and then optimized bufferization is trying to avoid the temporary array and use x as a direct storage of the function's result. You should be able to see the diference after OptimizedBufferization if you make x an assumed shape array.

[tblah]

LGTM, thanks

[jeanPerier]

Thank you Asher, LGTM