Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 17, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions flang/include/flang/Lower/FIRBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
namespace fir {
class AbstractArrayBox;
class ExtendedValue;
class IrBoxValue;
class BoxValue;
} // namespace fir

namespace Fortran::lower {
Expand Down Expand Up @@ -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);

//===--------------------------------------------------------------------===//
Expand Down
108 changes: 32 additions & 76 deletions flang/include/flang/Lower/Support/BoxValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &);

//===----------------------------------------------------------------------===//
//
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still used? (I thought we had removed it a while back.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is still used around subscript-triplet lowering here:

f18-llvm-project/flang/lib/Lower/ConvertExpr.cpp

Line 995 in 491e6f1

fir::RangeBoxValue genTriple(const Fortran::evaluate::Triplet &trip) {

It is a very localized use, so the definition could also be moved to ConvertExpr.cpp to avoid being exported in a public header.


class ExtendedValue;

mlir::Value getBase(const ExtendedValue &exv);
Expand All @@ -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;
Expand Down
19 changes: 5 additions & 14 deletions flang/lib/Lower/Allocatable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
7 changes: 5 additions & 2 deletions flang/lib/Lower/Bridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
23 changes: 6 additions & 17 deletions flang/lib/Lower/ConvertExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; });
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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());
},
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 8 additions & 8 deletions flang/lib/Lower/ConvertVariable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
Expand Down
Loading