Merge pull request #671 from flang-compiler/jp-boxvalue-clean-up

[NFC] Remove old BoxValue class and rename IrBoxValue to BoxValue

Author: jeanPerier

flang/include/flang/Lower/FIRBuilder.h

@@ -28,7 +28,7 @@
 namespace fir {
 class AbstractArrayBox;
 class ExtendedValue;
-class IrBoxValue;
+class BoxValue;
 } // namespace fir
 
 namespace Fortran::lower {
@@ -278,8 +278,8 @@ mlir::Value readLowerBound(FirOpBuilder &, mlir::Location,
                            const fir::ExtendedValue &, unsigned dim,
                            mlir::Value defaultValue);
 
-/// Read extents from an IrBoxValue into \p result.
-void readExtents(FirOpBuilder &, mlir::Location, const fir::IrBoxValue &,
+/// Read extents from an BoxValue into \p result.
+void readExtents(FirOpBuilder &, mlir::Location, const fir::BoxValue &,
                  llvm::SmallVectorImpl<mlir::Value> &result);
 
 //===--------------------------------------------------------------------===//

flang/include/flang/Lower/Support/BoxValue.h

@@ -27,18 +27,16 @@ namespace fir {
 class CharBoxValue;
 class ArrayBoxValue;
 class CharArrayBoxValue;
-class BoxValue;
 class ProcBoxValue;
 class MutableBoxValue;
-class IrBoxValue;
+class BoxValue;
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CharBoxValue &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const ArrayBoxValue &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CharArrayBoxValue &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const BoxValue &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const ProcBoxValue &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const MutableBoxValue &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const IrBoxValue &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const BoxValue &);
 
 //===----------------------------------------------------------------------===//
 //
@@ -184,47 +182,13 @@ class ProcBoxValue : public AbstractBox {
   mlir::Value hostContext;
 };
 
-/// In the generalized form, a boxed value can have a dynamic size, be an array
-/// with dynamic extents and lbounds, and take dynamic type parameters.
-class BoxValue : public AbstractBox, public AbstractArrayBox {
-public:
-  BoxValue(mlir::Value addr) : AbstractBox{addr}, AbstractArrayBox{} {}
-  BoxValue(mlir::Value addr, mlir::Value len)
-      : AbstractBox{addr}, AbstractArrayBox{}, len{len} {}
-  BoxValue(mlir::Value addr, llvm::ArrayRef<mlir::Value> extents,
-           llvm::ArrayRef<mlir::Value> lbounds = {})
-      : AbstractBox{addr}, AbstractArrayBox{extents, lbounds} {}
-  BoxValue(mlir::Value addr, mlir::Value len,
-           llvm::ArrayRef<mlir::Value> params,
-           llvm::ArrayRef<mlir::Value> extents,
-           llvm::ArrayRef<mlir::Value> lbounds = {})
-      : AbstractBox{addr}, AbstractArrayBox{extents, lbounds}, len{len},
-        params{params.begin(), params.end()} {}
-
-  BoxValue clone(mlir::Value newBase) const {
-    return {newBase, len, params, extents, lbounds};
-  }
-
-  BoxValue cloneElement(mlir::Value newBase) const {
-    return {newBase, len, params, {}, {}};
-  }
-
-  mlir::Value getLen() const { return len; }
-
-  llvm::ArrayRef<mlir::Value> getLenTypeParams() const { return params; }
-
-  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, const BoxValue &);
-  LLVM_DUMP_METHOD void dump() const { llvm::errs() << *this; }
-
-protected:
-  mlir::Value len;                          // box is CHARACTER
-  llvm::SmallVector<mlir::Value, 2> params; // LENs, box is derived type
-};
-
 /// Base class for values associated to a fir.box or fir.ref<fir.box>.
-class AbstractIrBox : public AbstractBox {
+class AbstractIrBox : public AbstractBox, public AbstractArrayBox {
 public:
   AbstractIrBox(mlir::Value addr) : AbstractBox{addr} {}
+  AbstractIrBox(mlir::Value addr, llvm::ArrayRef<mlir::Value> lbounds,
+                llvm::ArrayRef<mlir::Value> extents)
+      : AbstractBox{addr}, AbstractArrayBox(extents, lbounds) {}
   /// Get the fir.box<type> part of the address type.
   fir::BoxType getBoxTy() const {
     auto type = getAddr().getType();
@@ -255,8 +219,7 @@ class AbstractIrBox : public AbstractBox {
   /// Returns the rank of the entity. Beware that zero will be returned for
   /// both scalars and assumed rank.
   unsigned rank() const {
-    auto seqTy = getBaseTy().dyn_cast<fir::SequenceType>();
-    if (seqTy)
+    if (auto seqTy = getBaseTy().dyn_cast<fir::SequenceType>())
       return seqTy.getDimension();
     return 0;
   }
@@ -271,61 +234,52 @@ class AbstractIrBox : public AbstractBox {
 };
 
 /// An entity described by a fir.box value that cannot be read into
-/// another BoxValue category, either because the fir.box may be an
+/// another ExtendedValue category, either because the fir.box may be an
 /// absent optional and we need to wait until the user is referencing it
 /// to read it, or because it contains important information that cannot
 /// be exposed in FIR (e.g. non contiguous byte stride).
 /// It may also store explicit bounds or length parameters that were specified
 /// for the entity.
-class IrBoxValue : public AbstractIrBox {
+class BoxValue : public AbstractIrBox {
 public:
-  IrBoxValue(mlir::Value addr) : AbstractIrBox{addr} { assert(verify()); }
-  IrBoxValue(mlir::Value addr, llvm::ArrayRef<mlir::Value> lbounds,
-             llvm::ArrayRef<mlir::Value> explicitParams,
-             llvm::ArrayRef<mlir::Value> explicitExtents = {})
-      : AbstractIrBox{addr}, lbounds{lbounds.begin(), lbounds.end()},
-        explicitParams{explicitParams.begin(), explicitParams.end()},
-        explicitExtents{explicitExtents.begin(), explicitExtents.end()} {
+  BoxValue(mlir::Value addr) : AbstractIrBox{addr} { assert(verify()); }
+  BoxValue(mlir::Value addr, llvm::ArrayRef<mlir::Value> lbounds,
+           llvm::ArrayRef<mlir::Value> explicitParams,
+           llvm::ArrayRef<mlir::Value> explicitExtents = {})
+      : AbstractIrBox{addr, lbounds, explicitExtents},
+        explicitParams{explicitParams.begin(), explicitParams.end()} {
     assert(verify());
   }
   // TODO: check contiguous attribute of addr
   bool isContiguous() const { return false; }
 
-  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, const IrBoxValue &);
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &, const BoxValue &);
   LLVM_DUMP_METHOD void dump() const { llvm::errs() << *this; }
 
   llvm::ArrayRef<mlir::Value> getLBounds() const { return lbounds; }
-  llvm::ArrayRef<mlir::Value> getExplicitExtents() const {
-    return explicitExtents;
-  }
+
+  // The extents member is not guaranteed to be field for arrays. It is only
+  // guaranteed to be field for explicit shape arrays. In general,
+  // explicit-shape will not come as descriptors, so this field will be empty in
+  // most cases. The exception are derived types with length parameters and
+  // polymorphic dummy argument arrays. It may be possible for the explicit
+  // extents to conflict with the shape information that is in the box according
+  // to 15.5.2.11 sequence association rules.
+  llvm::ArrayRef<mlir::Value> getExplicitExtents() const { return extents; }
+
   llvm::ArrayRef<mlir::Value> getExplicitParameters() const {
     return explicitParams;
   }
 
 protected:
   // Verify constructor invariants.
   bool verify() const;
-  // Always field when the IrBoxValue has lower bounds other than one.
-  llvm::SmallVector<mlir::Value, 4> lbounds;
 
-  // Only field when the IrBoxValue has explicit length parameters.
+  // Only field when the BoxValue has explicit length parameters.
   // Otherwise, the length parameters are in the fir.box.
   llvm::SmallVector<mlir::Value, 2> explicitParams;
-
-  // Only field with the explicit length parameters
-  // Otherwise, the extents are in the fir.box.
-  llvm::SmallVector<mlir::Value, 4> explicitExtents;
-  // Note about explicitExtents: In general, explicit-shape will not come as
-  // descriptors, so this field will be empty in most cases. The exception are
-  // derived types with length parameters and polymorphic dummy argument arrays.
-  // It may be possible for the explicit extents to conflict with
-  // the shape information that is in the box according to 15.5.2.11
-  // sequence association rules.
 };
 
-/// Used for triple notation (array slices)
-using RangeBoxValue = std::tuple<mlir::Value, mlir::Value, mlir::Value>;
-
 /// Set of variables (addresses) holding the allocatable properties. These may
 /// be empty in case it is not deemed safe to duplicate the descriptor
 /// information locally (For instance, a volatile allocatable will always be
@@ -404,6 +358,9 @@ class MutableBoxValue : public AbstractIrBox {
   MutableProperties mutableProperties;
 };
 
+/// Used for triple notation (array slices)
+using RangeBoxValue = std::tuple<mlir::Value, mlir::Value, mlir::Value>;
+
 class ExtendedValue;
 
 mlir::Value getBase(const ExtendedValue &exv);
@@ -419,9 +376,8 @@ bool isArray(const ExtendedValue &exv);
 /// indices if it is an array entity.
 class ExtendedValue : public details::matcher<ExtendedValue> {
 public:
-  using VT =
-      std::variant<UnboxedValue, CharBoxValue, ArrayBoxValue, CharArrayBoxValue,
-                   BoxValue, ProcBoxValue, IrBoxValue>;
+  using VT = std::variant<UnboxedValue, CharBoxValue, ArrayBoxValue,
+                          CharArrayBoxValue, ProcBoxValue, BoxValue>;
 
   ExtendedValue() : box{UnboxedValue{}} {}
   ExtendedValue(const ExtendedValue &) = default;

flang/lib/Lower/Allocatable.cpp

@@ -954,9 +954,9 @@ Fortran::lower::createTempMutableBox(Fortran::lower::FirOpBuilder &builder,
 // MutableBoxValue reading interface implementation
 //===----------------------------------------------------------------------===//
 
-/// Helper to decide if a MutableBoxValue must be read to an IrBoxValue or
+/// Helper to decide if a MutableBoxValue must be read to an BoxValue or
 /// can be read to a reified box value.
-static bool readToIrBoxValue(const fir::MutableBoxValue &box) {
+static bool readToBoxValue(const fir::MutableBoxValue &box) {
   // If this is described by a set of local variables, the value
   // should not be tracked as a fir.box.
   if (box.isDescribedByVariables())
@@ -984,11 +984,11 @@ Fortran::lower::genMutableBoxRead(Fortran::lower::FirOpBuilder &builder,
   llvm::SmallVector<mlir::Value, 2> lbounds;
   llvm::SmallVector<mlir::Value, 2> extents;
   llvm::SmallVector<mlir::Value, 2> lengths;
-  if (readToIrBoxValue(box)) {
+  if (readToBoxValue(box)) {
     auto reader = MutablePropertyReader(builder, loc, box);
     reader.getLowerBounds(lbounds);
-    return fir::IrBoxValue{reader.getIrBox(), lbounds,
-                           box.nonDeferredLenParams()};
+    return fir::BoxValue{reader.getIrBox(), lbounds,
+                         box.nonDeferredLenParams()};
   }
   // Contiguous intrinsic type entity: all the data can be extracted from the
   // fir.box.
@@ -1046,11 +1046,6 @@ void Fortran::lower::associateMutableBoxWithShift(
                                 arr.getExtents(), {arr.getLen()});
       },
       [&](const fir::BoxValue &arr) {
-        writer.updateMutableBox(arr.getAddr(),
-                                lbounds.empty() ? arr.getLBounds() : lbounds,
-                                arr.getExtents(), arr.getLenTypeParams());
-      },
-      [&](const fir::IrBoxValue &arr) {
         // Rebox array fir.box to the pointer type and apply potential new lower
         // bounds.
         mlir::Value shift;
@@ -1129,10 +1124,6 @@ void Fortran::lower::associateMutableBoxWithRemap(
                                 {arr.getLen()});
       },
       [&](const fir::BoxValue &arr) {
-        writer.updateMutableBox(cast(arr.getAddr()), lbounds, extents,
-                                arr.getLenTypeParams());
-      },
-      [&](const fir::IrBoxValue &arr) {
         // Rebox right-hand side fir.box with a new shape and type.
         auto shapeType =
             fir::ShapeShiftType::get(builder.getContext(), extents.size());

flang/lib/Lower/Bridge.cpp

@@ -1250,8 +1250,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
                           sym, value.getAddr(), value.getLen(),
                           value.getExtents(), value.getLBounds());
                   },
-                  [&](const fir::BoxValue &) {
-                    TODO(toLocation(), "association selector of derived type");
+                  [&](const fir::BoxValue &value) {
+                    localSymbols.addBoxSymbol(sym, value.getAddr(),
+                                              value.getLBounds(),
+                                              value.getExplicitParameters(),
+                                              value.getExplicitExtents());
                   },
                   [&](const auto &) {
                     mlir::emitError(toLocation(),

flang/lib/Lower/ConvertExpr.cpp

@@ -182,16 +182,13 @@ arrayElementToExtendedValue(Fortran::lower::FirOpBuilder &builder,
       [&](const fir::CharArrayBoxValue &bv) -> fir::ExtendedValue {
         return bv.cloneElement(element);
       },
-      [&](const fir::BoxValue &bv) -> fir::ExtendedValue {
-        return bv.cloneElement(element);
-      },
-      [&](const fir::IrBoxValue &box) -> fir::ExtendedValue {
+      [&](const fir::BoxValue &box) -> fir::ExtendedValue {
         if (box.isCharacter()) {
           auto len = Fortran::lower::readCharLen(builder, loc, box);
           return fir::CharBoxValue{element, len};
         }
         if (box.isDerived())
-          TODO(loc, "get length parameters from IrBox");
+          TODO(loc, "get length parameters from derived type BoxValue");
         return element;
       },
       [&](const auto &) -> fir::ExtendedValue { return element; });
@@ -1188,16 +1185,12 @@ class ScalarExprLowering {
             delta = one;
           return fir::CharBoxValue(genFullDim(arr, delta), arr.getLen());
         },
-        [&](const Fortran::lower::SymbolBox::Derived &arr)
-            -> fir::ExtendedValue {
-          TODO(loc, "array ref of derived type with length parameters");
-        },
-        [&](const Fortran::lower::SymbolBox::IrBox &arr) -> fir::ExtendedValue {
-          // CoordinateOp for IrBoxValue is not generated here. The dimensions
+        [&](const Fortran::lower::SymbolBox::Box &arr) -> fir::ExtendedValue {
+          // CoordinateOp for BoxValue is not generated here. The dimensions
           // must be kept in the fir.coordinate_op so that potential fir.box
           // strides can be applied by codegen.
           fir::emitFatalError(
-              loc, "internal: IrBoxValue in dim-collapsed fir.coordinate_of");
+              loc, "internal: BoxValue in dim-collapsed fir.coordinate_of");
         },
         [&](const auto &) -> fir::ExtendedValue {
           fir::emitFatalError(loc, "internal: array lowering failed");
@@ -1235,10 +1228,6 @@ class ScalarExprLowering {
           lengthParams.emplace_back(arr.getLen());
         },
         [&](const fir::BoxValue &arr) {
-          auto lengths = arr.getLenTypeParams();
-          lengthParams.append(lengths.begin(), lengths.end());
-        },
-        [&](const fir::IrBoxValue &arr) {
           auto lengths = arr.getExplicitParameters();
           lengthParams.append(lengths.begin(), lengths.end());
         },
@@ -2468,7 +2457,7 @@ class ArrayExprLowering {
         TODO(loc, "use fir.rebox for array section of fir.box");
       mlir::Value embox = builder.create<fir::EmboxOp>(
           loc, boxTy, memref, shape, slice, /*lenParams=*/llvm::None);
-      return [=](IterSpace) -> ExtValue { return fir::IrBoxValue(embox); };
+      return [=](IterSpace) -> ExtValue { return fir::BoxValue(embox); };
     }
     mlir::Value arrLd = builder.create<fir::ArrayLoadOp>(
         loc, arrTy, memref, shape, slice, /*lenParams=*/llvm::None);

flang/lib/Lower/ConvertVariable.cpp

@@ -681,13 +681,13 @@ static void instantiateCommon(Fortran::lower::AbstractConverter &converter,
 // Lower Variables specification expressions and attributes
 //===--------------------------------------------------------------===//
 
-/// Helper to decide if a dummy argument must be tracked in an IrBox.
-static bool lowerToIrBox(const Fortran::semantics::Symbol &sym,
-                         mlir::Value dummyArg) {
-  // Only dummy arguments coming as fir.box can be tracked in an IrBox.
+/// Helper to decide if a dummy argument must be tracked in an BoxValue.
+static bool lowerToBoxValue(const Fortran::semantics::Symbol &sym,
+                            mlir::Value dummyArg) {
+  // Only dummy arguments coming as fir.box can be tracked in an BoxValue.
   if (!dummyArg || !dummyArg.getType().isa<fir::BoxType>())
     return false;
-  // Non contiguous arrays must be tracked in an IrBox.
+  // Non contiguous arrays must be tracked in an BoxValue.
   if (sym.Rank() > 0 && !sym.attrs().test(Fortran::semantics::Attr::CONTIGUOUS))
     return true;
   // Assumed rank and optional fir.box cannot yet be read while lowering the
@@ -855,7 +855,7 @@ void Fortran::lower::mapSymbolAttributes(
 
   if (isDummy) {
     auto dummyArg = symMap.lookupSymbol(sym).getAddr();
-    if (lowerToIrBox(sym, dummyArg)) {
+    if (lowerToBoxValue(sym, dummyArg)) {
       llvm::SmallVector<mlir::Value, 4> lbounds;
       llvm::SmallVector<mlir::Value, 4> extents;
       llvm::SmallVector<mlir::Value, 2> explicitParams;
@@ -869,8 +869,8 @@ void Fortran::lower::mapSymbolAttributes(
       lowerExplicitLowerBounds(converter, loc, sba, lbounds, symMap, stmtCtx);
       lowerExplicitExtents(converter, loc, sba, lbounds, extents, symMap,
                            stmtCtx);
-      symMap.addIrBoxSymbol(sym, dummyArg, lbounds, explicitParams, extents,
-                            replace);
+      symMap.addBoxSymbol(sym, dummyArg, lbounds, explicitParams, extents,
+                          replace);
       return;
     }
   }