[flang] update fir.coordinate_of to carry the fields #127231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeanPerier
requested review from
razvanlupusoru,
rscottmanley,
clementval,
tblah and
vzakhari
|
Thanks for the patch @jeanPerier! The intent to remove such operation looks good to me. |
|
Nice work! |
clementval
approved these changes
Feb 23, 2025
tblah
reviewed
Feb 24, 2025
jeanPerier
force-pushed
the
users/jeanPerier/no-field-index
branch
from
e6a487e to
538552d
Compare
jeanPerier
changed the title
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-codegen @llvm/pr-subscribers-flang-fir-hlfir Author: None (jeanPerier) ChangesThis patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands. The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible. This make IR transformation harder/brittle, so I want to update FIR to get rid if this. Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change. After this patch similar changes should be done to make FIR more robust:
Patch is 424.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127231.diff 68 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index a21f8bbe17685..ed301016ad01c 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -50,9 +50,95 @@ struct DebuggingResource
mlir::StringRef getName() final { return "DebuggingResource"; }
};
+class CoordinateIndicesAdaptor;
+using IntOrValue = llvm::PointerUnion<mlir::IntegerAttr, mlir::Value>;
+
} // namespace fir
#define GET_OP_CLASSES
#include "flang/Optimizer/Dialect/FIROps.h.inc"
+namespace fir {
+class CoordinateIndicesAdaptor {
+public:
+ using value_type = IntOrValue;
+
+ CoordinateIndicesAdaptor(mlir::DenseI32ArrayAttr fieldIndices,
+ mlir::ValueRange values)
+ : fieldIndices(fieldIndices), values(values) {}
+
+ value_type operator[](size_t index) const {
+ assert(index < size() && "index out of bounds");
+ return *std::next(begin(), index);
+ }
+
+ size_t size() const {
+ return fieldIndices ? fieldIndices.size() : values.size();
+ }
+
+ bool empty() const {
+ return values.empty() && (!fieldIndices || fieldIndices.empty());
+ }
+
+ class iterator
+ : public llvm::iterator_facade_base<iterator, std::forward_iterator_tag,
+ value_type, std::ptrdiff_t,
+ value_type *, value_type> {
+ public:
+ iterator(const CoordinateIndicesAdaptor *base,
+ std::optional<llvm::ArrayRef<int32_t>::iterator> fieldIter,
+ llvm::detail::IterOfRange<const mlir::ValueRange> valuesIter)
+ : base(base), fieldIter(fieldIter), valuesIter(valuesIter) {}
+
+ value_type operator*() const {
+ if (fieldIter && **fieldIter != fir::CoordinateOp::kDynamicIndex) {
+ return mlir::IntegerAttr::get(base->fieldIndices.getElementType(),
+ **fieldIter);
+ }
+ return *valuesIter;
+ }
+
+ iterator &operator++() {
+ if (fieldIter) {
+ if (**fieldIter == fir::CoordinateOp::kDynamicIndex)
+ valuesIter++;
+ (*fieldIter)++;
+ } else {
+ valuesIter++;
+ }
+ return *this;
+ }
+
+ bool operator==(const iterator &rhs) const {
+ return base == rhs.base && fieldIter == rhs.fieldIter &&
+ valuesIter == rhs.valuesIter;
+ }
+
+ private:
+ const CoordinateIndicesAdaptor *base;
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ llvm::detail::IterOfRange<const mlir::ValueRange> valuesIter;
+ };
+
+ iterator begin() const {
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ if (fieldIndices)
+ fieldIter = fieldIndices.asArrayRef().begin();
+ return iterator(this, fieldIter, values.begin());
+ }
+
+ iterator end() const {
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ if (fieldIndices)
+ fieldIter = fieldIndices.asArrayRef().end();
+ return iterator(this, fieldIter, values.end());
+ }
+
+private:
+ mlir::DenseI32ArrayAttr fieldIndices;
+ mlir::ValueRange values;
+};
+
+} // namespace fir
+
#endif // FORTRAN_OPTIMIZER_DIALECT_FIROPS_H
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 8dbc9df9f553d..c83c57186b46d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -1748,10 +1748,16 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
Unlike LLVM's GEP instruction, one cannot stride over the outermost
reference; therefore, the leading 0 index must be omitted.
+ This operation can be used to index derived type fields, in which case
+ the operand is the name of the index field.
+
```
%i = ... : index
%h = ... : !fir.heap<!fir.array<100 x f32>>
%p = fir.coordinate_of %h, %i : (!fir.heap<!fir.array<100 x f32>>, index) -> !fir.ref<f32>
+
+ %d = ... : !fir.ref<!fir.type<t{field1:i32, field2:f32}>>
+ %f = fir.coordinate_of %d, field2 : (!fir.ref<!fir.type<t{field1:i32, field2:f32}>>) -> !fir.ref<f32>
```
In the example, `%p` will be a pointer to the `%i`-th f32 value in the
@@ -1761,7 +1767,8 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
let arguments = (ins
AnyRefOrBox:$ref,
Variadic<AnyCoordinateType>:$coor,
- TypeAttr:$baseType
+ TypeAttr:$baseType,
+ OptionalAttr<DenseI32ArrayAttr>:$field_indices
);
let results = (outs RefOrLLVMPtr);
@@ -1771,10 +1778,14 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
let builders = [
OpBuilder<(ins "mlir::Type":$resultType,
- "mlir::Value":$ref, "mlir::ValueRange":$coor),
- [{ return build($_builder, $_state, resultType, ref, coor,
- mlir::TypeAttr::get(ref.getType())); }]>,
+ "mlir::Value":$ref, "mlir::ValueRange":$coor)>,
+ OpBuilder<(ins "mlir::Type":$resultType,
+ "mlir::Value":$ref, "llvm::ArrayRef<fir::IntOrValue>":$coor)>
];
+ let extraClassDeclaration = [{
+ constexpr static int32_t kDynamicIndex = std::numeric_limits<int32_t>::min();
+ CoordinateIndicesAdaptor getIndices();
+ }];
}
def fir_ExtractValueOp : fir_OneResultOp<"extract_value", [NoMemoryEffect]> {
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index fa1975dac789b..48bcf492fd368 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -354,14 +354,12 @@ mlir::Value createParentSymAndGenIntermediateMaps(
// type.
if (fir::RecordType recordType = mlir::dyn_cast<fir::RecordType>(
fir::unwrapPassByRefType(curValue.getType()))) {
- mlir::Value idxConst = firOpBuilder.createIntegerConstant(
- clauseLocation, firOpBuilder.getIndexType(),
- indices[currentIndicesIdx]);
- mlir::Type memberTy =
- recordType.getTypeList().at(indices[currentIndicesIdx]).second;
+ fir::IntOrValue idxConst = mlir::IntegerAttr::get(
+ firOpBuilder.getI32Type(), indices[currentIndicesIdx]);
+ mlir::Type memberTy = recordType.getType(indices[currentIndicesIdx]);
curValue = firOpBuilder.create<fir::CoordinateOp>(
clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
- idxConst);
+ llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
// Skip mapping and the subsequent load if we're the final member or not
// a type with a descriptor such as a pointer/allocatable. If we're a
diff --git a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
index 26f4aee21d8bd..82b11ad7db32a 100644
--- a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
+++ b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
@@ -348,8 +348,9 @@ class BoxedProcedurePass
rewriter.setInsertionPoint(coor);
auto toTy = typeConverter.convertType(ty);
auto toBaseTy = typeConverter.convertType(baseTy);
- rewriter.replaceOpWithNewOp<CoordinateOp>(coor, toTy, coor.getRef(),
- coor.getCoor(), toBaseTy);
+ rewriter.replaceOpWithNewOp<CoordinateOp>(
+ coor, toTy, coor.getRef(), coor.getCoor(), toBaseTy,
+ coor.getFieldIndicesAttr());
opIsValid = false;
}
} else if (auto index = mlir::dyn_cast<FieldIndexOp>(op)) {
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index aaefe675730e1..a2743edd7844a 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2653,57 +2653,78 @@ struct CoordinateOpConversion
return mlir::isa<fir::SequenceType, fir::RecordType, mlir::TupleType>(type);
}
- /// Check whether this form of `!fir.coordinate_of` is supported. These
- /// additional checks are required, because we are not yet able to convert
- /// all valid forms of `!fir.coordinate_of`.
- /// TODO: Either implement the unsupported cases or extend the verifier
- /// in FIROps.cpp instead.
- static bool supportedCoordinate(mlir::Type type, mlir::ValueRange coors) {
- const std::size_t numOfCoors = coors.size();
- std::size_t i = 0;
- bool subEle = false;
- bool ptrEle = false;
- for (; i < numOfCoors; ++i) {
- mlir::Value nxtOpnd = coors[i];
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(type)) {
- subEle = true;
- i += arrTy.getDimension() - 1;
- type = arrTy.getEleTy();
- } else if (auto recTy = mlir::dyn_cast<fir::RecordType>(type)) {
- subEle = true;
- type = recTy.getType(getFieldNumber(recTy, nxtOpnd));
- } else if (auto tupTy = mlir::dyn_cast<mlir::TupleType>(type)) {
- subEle = true;
- type = tupTy.getType(getConstantIntValue(nxtOpnd));
- } else {
- ptrEle = true;
- }
- }
- if (ptrEle)
- return (!subEle) && (numOfCoors == 1);
- return subEle && (i >= numOfCoors);
- }
+ // Helper structure to analyze the CoordinateOp path and decide if and how
+ // the GEP should be generated for it.
+ struct ShapeAnalysis {
+ bool hasKnownShape;
+ bool columnIsDeferred;
+ };
/// Walk the abstract memory layout and determine if the path traverses any
/// array types with unknown shape. Return true iff all the array types have a
/// constant shape along the path.
- static bool arraysHaveKnownShape(mlir::Type type, mlir::ValueRange coors) {
- for (std::size_t i = 0, sz = coors.size(); i < sz; ++i) {
- mlir::Value nxtOpnd = coors[i];
+ /// TODO: move the verification logic into the verifier.
+ static std::optional<ShapeAnalysis>
+ arraysHaveKnownShape(mlir::Type type, fir::CoordinateOp coor) {
+ fir::CoordinateIndicesAdaptor indices = coor.getIndices();
+ auto begin = indices.begin();
+ bool hasKnownShape = true;
+ bool columnIsDeferred = false;
+ for (auto it = begin, end = indices.end(); it != end;) {
if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(type)) {
- if (fir::sequenceWithNonConstantShape(arrTy))
- return false;
- i += arrTy.getDimension() - 1;
+ bool addressingStart = (it == begin);
+ unsigned arrayDim = arrTy.getDimension();
+ for (auto dimExtent : llvm::enumerate(arrTy.getShape())) {
+ if (dimExtent.value() == fir::SequenceType::getUnknownExtent()) {
+ hasKnownShape = false;
+ if (addressingStart && dimExtent.index() + 1 == arrayDim) {
+ // If this point was reached, the raws of the first array have
+ // constant extents.
+ columnIsDeferred = true;
+ } else {
+ // One of the array dimension that is not the column of the first
+ // array has dynamic extent. It will not possible to do
+ // code generation for the CoordinateOp if the base is not a
+ // fir.box containing the value of that extent.
+ return ShapeAnalysis{false, false};
+ }
+ }
+ // There may be less operands than the array size if the
+ // fir.coordinate_of result is not an element but a sub-array.
+ if (it != end)
+ ++it;
+ }
type = arrTy.getEleTy();
- } else if (auto strTy = mlir::dyn_cast<fir::RecordType>(type)) {
- type = strTy.getType(getFieldNumber(strTy, nxtOpnd));
+ continue;
+ }
+ if (auto strTy = mlir::dyn_cast<fir::RecordType>(type)) {
+ auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(*it);
+ if (!intAttr) {
+ mlir::emitError(coor.getLoc(),
+ "expected field name in fir.coordinate_of");
+ return std::nullopt;
+ }
+ type = strTy.getType(intAttr.getInt());
} else if (auto strTy = mlir::dyn_cast<mlir::TupleType>(type)) {
- type = strTy.getType(getConstantIntValue(nxtOpnd));
- } else {
- return true;
+ auto value = llvm::dyn_cast<mlir::Value>(*it);
+ if (!value) {
+ mlir::emitError(
+ coor.getLoc(),
+ "expected constant value to address tuple in fir.coordinate_of");
+ return std::nullopt;
+ }
+ type = strTy.getType(getConstantIntValue(value));
+ } else if (auto charType = mlir::dyn_cast<fir::CharacterType>(type)) {
+ // Addressing character in string. Fortran strings degenerate to arrays
+ // in LLVM, so they are handled like arrays of characters here.
+ if (charType.getLen() == fir::CharacterType::unknownLen())
+ return ShapeAnalysis{false, true};
+ type = fir::CharacterType::getSingleton(charType.getContext(),
+ charType.getFKind());
}
+ ++it;
}
- return true;
+ return ShapeAnalysis{hasKnownShape, columnIsDeferred};
}
private:
@@ -2754,9 +2775,11 @@ struct CoordinateOpConversion
mlir::LLVM::IntegerOverflowFlags nsw =
mlir::LLVM::IntegerOverflowFlags::nsw;
- for (unsigned i = 1, last = operands.size(); i < last; ++i) {
+ int nextIndexValue = 1;
+ fir::CoordinateIndicesAdaptor indices = coor.getIndices();
+ for (auto it = indices.begin(), end = indices.end(); it != end;) {
if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- if (i != 1)
+ if (it != indices.begin())
TODO(loc, "fir.array nested inside other array and/or derived type");
// Applies byte strides from the box. Ignore lower bound from box
// since fir.coordinate_of indexes are zero based. Lowering takes care
@@ -2764,26 +2787,31 @@ struct CoordinateOpConversion
// types and non contiguous arrays.
auto idxTy = lowerTy().indexType();
mlir::Value off = genConstantIndex(loc, idxTy, rewriter, 0);
- for (unsigned index = i, lastIndex = i + arrTy.getDimension();
- index < lastIndex; ++index) {
- mlir::Value stride = getStrideFromBox(loc, boxTyPair, operands[0],
- index - i, rewriter);
+ unsigned arrayDim = arrTy.getDimension();
+ for (unsigned dim = 0; dim < arrayDim && it != end; ++dim, ++it) {
+ mlir::Value stride =
+ getStrideFromBox(loc, boxTyPair, operands[0], dim, rewriter);
auto sc = rewriter.create<mlir::LLVM::MulOp>(
- loc, idxTy, operands[index], stride, nsw);
+ loc, idxTy, operands[nextIndexValue + dim], stride, nsw);
off = rewriter.create<mlir::LLVM::AddOp>(loc, idxTy, sc, off, nsw);
}
+ nextIndexValue += arrayDim;
resultAddr = rewriter.create<mlir::LLVM::GEPOp>(
loc, llvmPtrTy, byteTy, resultAddr,
llvm::ArrayRef<mlir::LLVM::GEPArg>{off});
- i += arrTy.getDimension() - 1;
cpnTy = arrTy.getEleTy();
} else if (auto recTy = mlir::dyn_cast<fir::RecordType>(cpnTy)) {
- mlir::Value nxtOpnd = operands[i];
- cpnTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+ auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(*it);
+ if (!intAttr)
+ return mlir::emitError(loc,
+ "expected field name in fir.coordinate_of");
+ int fieldIndex = intAttr.getInt();
+ ++it;
+ cpnTy = recTy.getType(fieldIndex);
auto llvmRecTy = lowerTy().convertType(recTy);
resultAddr = rewriter.create<mlir::LLVM::GEPOp>(
loc, llvmPtrTy, llvmRecTy, resultAddr,
- llvm::ArrayRef<mlir::LLVM::GEPArg>{0, nxtOpnd});
+ llvm::ArrayRef<mlir::LLVM::GEPArg>{0, fieldIndex});
} else {
fir::emitFatalError(loc, "unexpected type in coordinate_of");
}
@@ -2801,92 +2829,71 @@ struct CoordinateOpConversion
// Component Type
mlir::Type cpnTy = fir::dyn_cast_ptrOrBoxEleTy(baseObjectTy);
- bool hasSubdimension = hasSubDimensions(cpnTy);
- bool columnIsDeferred = !hasSubdimension;
-
- if (!supportedCoordinate(cpnTy, operands.drop_front(1)))
- TODO(loc, "unsupported combination of coordinate operands");
-
- const bool hasKnownShape =
- arraysHaveKnownShape(cpnTy, operands.drop_front(1));
-
- // If only the column is `?`, then we can simply place the column value in
- // the 0-th GEP position.
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- if (!hasKnownShape) {
- const unsigned sz = arrTy.getDimension();
- if (arraysHaveKnownShape(arrTy.getEleTy(),
- operands.drop_front(1 + sz))) {
- fir::SequenceType::ShapeRef shape = arrTy.getShape();
- bool allConst = true;
- for (unsigned i = 0; i < sz - 1; ++i) {
- if (shape[i] < 0) {
- allConst = false;
- break;
- }
- }
- if (allConst)
- columnIsDeferred = true;
- }
- }
- }
+
+ const std::optional<ShapeAnalysis> shapeAnalysis =
+ arraysHaveKnownShape(cpnTy, coor);
+ if (!shapeAnalysis)
+ return mlir::failure();
if (fir::hasDynamicSize(fir::unwrapSequenceType(cpnTy)))
return mlir::emitError(
loc, "fir.coordinate_of with a dynamic element size is unsupported");
- if (hasKnownShape || columnIsDeferred) {
+ if (shapeAnalysis->hasKnownShape || shapeAnalysis->columnIsDeferred) {
llvm::SmallVector<mlir::LLVM::GEPArg> offs;
- if (hasKnownShape && hasSubdimension) {
+ if (shapeAnalysis->hasKnownShape) {
offs.push_back(0);
}
+ // Else, only the column is `?` and we can simply place the column value
+ // in the 0-th GEP position.
+
std::optional<int> dims;
llvm::SmallVector<mlir::Value> arrIdx;
- for (std::size_t i = 1, sz = operands.size(); i < sz; ++i) {
- mlir::Value nxtOpnd = operands[i];
-
- if (!cpnTy)
- return mlir::emitError(loc, "invalid coordinate/check failed");
-
- // check if the i-th coordinate relates to an array
- if (dims) {
- arrIdx.push_back(nxtOpnd);
- int dimsLeft = *dims;
- if (dimsLeft > 1) {
- dims = dimsLeft - 1;
- continue;
- }
- cpnTy = mlir::cast<fir::SequenceType>(cpnTy).getElementType();
- // append array range in reverse (FIR arrays are column-major)
- offs.append(arrIdx.rbegin(), arrIdx.rend());
- arrIdx.clear();
- dims.reset();
+ int nextIndexValue = 1;
+ for (auto index : coor.getIndices()) {
+ if (auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(index)) {
+ // Addressing derived type component.
+ auto recordType = llvm::dyn_cast<fir::RecordType>(cpnTy);
+ if (!recordType)
+ return mlir::emitError(
+ loc,
+ "fir.coordinate base type is not consistent with operands");
+ int fieldId = intAttr.getInt();
+ cpnTy = recordType.getType(fieldId);
+ offs.push_back(fieldId);
continue;
}
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- int d = arrTy.getDimension() - 1;
- if (d > 0) {
- dims = d;
- arrIdx.push_back(nxtOpnd);
- continue;
+ // Value index (addressing array, tuple, or complex part).
+ mlir::Value indexValue = operands[nextIndexValue++];
+ if (auto tupTy = mlir::dyn_cast<mlir::TupleType>(cpnTy)) {
+ cpnTy = tupTy.getType(getConstantIntValue(indexValue));
+ offs.push_back(indexValue);
+ } else {
+ if (!dims) {
+ if (auto arrayType = llvm::dyn_cast<fir::SequenceType>(cpnTy)) {
+ // Starting addressing array or array component.
+ dims = arrayType.getDimension();
+ cpnTy = arrayType.getElementType();
+ }
+ }
+ if (dims) {
+ arrIdx.push_back(indexValue);
+ if (--(*dims) == 0) {
+ // Append array range in reverse (FIR arrays are column-major).
+ offs.append(arrIdx.rbegin(), arrIdx.rend());
+ arrIdx.clear();
+ dims.reset();
+ ...
[truncated]
|
|
@llvm/pr-subscribers-flang-openmp Author: None (jeanPerier) ChangesThis patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands. The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible. This make IR transformation harder/brittle, so I want to update FIR to get rid if this. Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change. After this patch similar changes should be done to make FIR more robust:
Patch is 424.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127231.diff 68 Files Affected:
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.h b/flang/include/flang/Optimizer/Dialect/FIROps.h
index a21f8bbe17685..ed301016ad01c 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.h
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.h
@@ -50,9 +50,95 @@ struct DebuggingResource
mlir::StringRef getName() final { return "DebuggingResource"; }
};
+class CoordinateIndicesAdaptor;
+using IntOrValue = llvm::PointerUnion<mlir::IntegerAttr, mlir::Value>;
+
} // namespace fir
#define GET_OP_CLASSES
#include "flang/Optimizer/Dialect/FIROps.h.inc"
+namespace fir {
+class CoordinateIndicesAdaptor {
+public:
+ using value_type = IntOrValue;
+
+ CoordinateIndicesAdaptor(mlir::DenseI32ArrayAttr fieldIndices,
+ mlir::ValueRange values)
+ : fieldIndices(fieldIndices), values(values) {}
+
+ value_type operator[](size_t index) const {
+ assert(index < size() && "index out of bounds");
+ return *std::next(begin(), index);
+ }
+
+ size_t size() const {
+ return fieldIndices ? fieldIndices.size() : values.size();
+ }
+
+ bool empty() const {
+ return values.empty() && (!fieldIndices || fieldIndices.empty());
+ }
+
+ class iterator
+ : public llvm::iterator_facade_base<iterator, std::forward_iterator_tag,
+ value_type, std::ptrdiff_t,
+ value_type *, value_type> {
+ public:
+ iterator(const CoordinateIndicesAdaptor *base,
+ std::optional<llvm::ArrayRef<int32_t>::iterator> fieldIter,
+ llvm::detail::IterOfRange<const mlir::ValueRange> valuesIter)
+ : base(base), fieldIter(fieldIter), valuesIter(valuesIter) {}
+
+ value_type operator*() const {
+ if (fieldIter && **fieldIter != fir::CoordinateOp::kDynamicIndex) {
+ return mlir::IntegerAttr::get(base->fieldIndices.getElementType(),
+ **fieldIter);
+ }
+ return *valuesIter;
+ }
+
+ iterator &operator++() {
+ if (fieldIter) {
+ if (**fieldIter == fir::CoordinateOp::kDynamicIndex)
+ valuesIter++;
+ (*fieldIter)++;
+ } else {
+ valuesIter++;
+ }
+ return *this;
+ }
+
+ bool operator==(const iterator &rhs) const {
+ return base == rhs.base && fieldIter == rhs.fieldIter &&
+ valuesIter == rhs.valuesIter;
+ }
+
+ private:
+ const CoordinateIndicesAdaptor *base;
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ llvm::detail::IterOfRange<const mlir::ValueRange> valuesIter;
+ };
+
+ iterator begin() const {
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ if (fieldIndices)
+ fieldIter = fieldIndices.asArrayRef().begin();
+ return iterator(this, fieldIter, values.begin());
+ }
+
+ iterator end() const {
+ std::optional<llvm::ArrayRef<int32_t>::const_iterator> fieldIter;
+ if (fieldIndices)
+ fieldIter = fieldIndices.asArrayRef().end();
+ return iterator(this, fieldIter, values.end());
+ }
+
+private:
+ mlir::DenseI32ArrayAttr fieldIndices;
+ mlir::ValueRange values;
+};
+
+} // namespace fir
+
#endif // FORTRAN_OPTIMIZER_DIALECT_FIROPS_H
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 8dbc9df9f553d..c83c57186b46d 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -1748,10 +1748,16 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
Unlike LLVM's GEP instruction, one cannot stride over the outermost
reference; therefore, the leading 0 index must be omitted.
+ This operation can be used to index derived type fields, in which case
+ the operand is the name of the index field.
+
```
%i = ... : index
%h = ... : !fir.heap<!fir.array<100 x f32>>
%p = fir.coordinate_of %h, %i : (!fir.heap<!fir.array<100 x f32>>, index) -> !fir.ref<f32>
+
+ %d = ... : !fir.ref<!fir.type<t{field1:i32, field2:f32}>>
+ %f = fir.coordinate_of %d, field2 : (!fir.ref<!fir.type<t{field1:i32, field2:f32}>>) -> !fir.ref<f32>
```
In the example, `%p` will be a pointer to the `%i`-th f32 value in the
@@ -1761,7 +1767,8 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
let arguments = (ins
AnyRefOrBox:$ref,
Variadic<AnyCoordinateType>:$coor,
- TypeAttr:$baseType
+ TypeAttr:$baseType,
+ OptionalAttr<DenseI32ArrayAttr>:$field_indices
);
let results = (outs RefOrLLVMPtr);
@@ -1771,10 +1778,14 @@ def fir_CoordinateOp : fir_Op<"coordinate_of", [NoMemoryEffect]> {
let builders = [
OpBuilder<(ins "mlir::Type":$resultType,
- "mlir::Value":$ref, "mlir::ValueRange":$coor),
- [{ return build($_builder, $_state, resultType, ref, coor,
- mlir::TypeAttr::get(ref.getType())); }]>,
+ "mlir::Value":$ref, "mlir::ValueRange":$coor)>,
+ OpBuilder<(ins "mlir::Type":$resultType,
+ "mlir::Value":$ref, "llvm::ArrayRef<fir::IntOrValue>":$coor)>
];
+ let extraClassDeclaration = [{
+ constexpr static int32_t kDynamicIndex = std::numeric_limits<int32_t>::min();
+ CoordinateIndicesAdaptor getIndices();
+ }];
}
def fir_ExtractValueOp : fir_OneResultOp<"extract_value", [NoMemoryEffect]> {
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index fa1975dac789b..48bcf492fd368 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -354,14 +354,12 @@ mlir::Value createParentSymAndGenIntermediateMaps(
// type.
if (fir::RecordType recordType = mlir::dyn_cast<fir::RecordType>(
fir::unwrapPassByRefType(curValue.getType()))) {
- mlir::Value idxConst = firOpBuilder.createIntegerConstant(
- clauseLocation, firOpBuilder.getIndexType(),
- indices[currentIndicesIdx]);
- mlir::Type memberTy =
- recordType.getTypeList().at(indices[currentIndicesIdx]).second;
+ fir::IntOrValue idxConst = mlir::IntegerAttr::get(
+ firOpBuilder.getI32Type(), indices[currentIndicesIdx]);
+ mlir::Type memberTy = recordType.getType(indices[currentIndicesIdx]);
curValue = firOpBuilder.create<fir::CoordinateOp>(
clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
- idxConst);
+ llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
// Skip mapping and the subsequent load if we're the final member or not
// a type with a descriptor such as a pointer/allocatable. If we're a
diff --git a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
index 26f4aee21d8bd..82b11ad7db32a 100644
--- a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
+++ b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
@@ -348,8 +348,9 @@ class BoxedProcedurePass
rewriter.setInsertionPoint(coor);
auto toTy = typeConverter.convertType(ty);
auto toBaseTy = typeConverter.convertType(baseTy);
- rewriter.replaceOpWithNewOp<CoordinateOp>(coor, toTy, coor.getRef(),
- coor.getCoor(), toBaseTy);
+ rewriter.replaceOpWithNewOp<CoordinateOp>(
+ coor, toTy, coor.getRef(), coor.getCoor(), toBaseTy,
+ coor.getFieldIndicesAttr());
opIsValid = false;
}
} else if (auto index = mlir::dyn_cast<FieldIndexOp>(op)) {
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index aaefe675730e1..a2743edd7844a 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2653,57 +2653,78 @@ struct CoordinateOpConversion
return mlir::isa<fir::SequenceType, fir::RecordType, mlir::TupleType>(type);
}
- /// Check whether this form of `!fir.coordinate_of` is supported. These
- /// additional checks are required, because we are not yet able to convert
- /// all valid forms of `!fir.coordinate_of`.
- /// TODO: Either implement the unsupported cases or extend the verifier
- /// in FIROps.cpp instead.
- static bool supportedCoordinate(mlir::Type type, mlir::ValueRange coors) {
- const std::size_t numOfCoors = coors.size();
- std::size_t i = 0;
- bool subEle = false;
- bool ptrEle = false;
- for (; i < numOfCoors; ++i) {
- mlir::Value nxtOpnd = coors[i];
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(type)) {
- subEle = true;
- i += arrTy.getDimension() - 1;
- type = arrTy.getEleTy();
- } else if (auto recTy = mlir::dyn_cast<fir::RecordType>(type)) {
- subEle = true;
- type = recTy.getType(getFieldNumber(recTy, nxtOpnd));
- } else if (auto tupTy = mlir::dyn_cast<mlir::TupleType>(type)) {
- subEle = true;
- type = tupTy.getType(getConstantIntValue(nxtOpnd));
- } else {
- ptrEle = true;
- }
- }
- if (ptrEle)
- return (!subEle) && (numOfCoors == 1);
- return subEle && (i >= numOfCoors);
- }
+ // Helper structure to analyze the CoordinateOp path and decide if and how
+ // the GEP should be generated for it.
+ struct ShapeAnalysis {
+ bool hasKnownShape;
+ bool columnIsDeferred;
+ };
/// Walk the abstract memory layout and determine if the path traverses any
/// array types with unknown shape. Return true iff all the array types have a
/// constant shape along the path.
- static bool arraysHaveKnownShape(mlir::Type type, mlir::ValueRange coors) {
- for (std::size_t i = 0, sz = coors.size(); i < sz; ++i) {
- mlir::Value nxtOpnd = coors[i];
+ /// TODO: move the verification logic into the verifier.
+ static std::optional<ShapeAnalysis>
+ arraysHaveKnownShape(mlir::Type type, fir::CoordinateOp coor) {
+ fir::CoordinateIndicesAdaptor indices = coor.getIndices();
+ auto begin = indices.begin();
+ bool hasKnownShape = true;
+ bool columnIsDeferred = false;
+ for (auto it = begin, end = indices.end(); it != end;) {
if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(type)) {
- if (fir::sequenceWithNonConstantShape(arrTy))
- return false;
- i += arrTy.getDimension() - 1;
+ bool addressingStart = (it == begin);
+ unsigned arrayDim = arrTy.getDimension();
+ for (auto dimExtent : llvm::enumerate(arrTy.getShape())) {
+ if (dimExtent.value() == fir::SequenceType::getUnknownExtent()) {
+ hasKnownShape = false;
+ if (addressingStart && dimExtent.index() + 1 == arrayDim) {
+ // If this point was reached, the raws of the first array have
+ // constant extents.
+ columnIsDeferred = true;
+ } else {
+ // One of the array dimension that is not the column of the first
+ // array has dynamic extent. It will not possible to do
+ // code generation for the CoordinateOp if the base is not a
+ // fir.box containing the value of that extent.
+ return ShapeAnalysis{false, false};
+ }
+ }
+ // There may be less operands than the array size if the
+ // fir.coordinate_of result is not an element but a sub-array.
+ if (it != end)
+ ++it;
+ }
type = arrTy.getEleTy();
- } else if (auto strTy = mlir::dyn_cast<fir::RecordType>(type)) {
- type = strTy.getType(getFieldNumber(strTy, nxtOpnd));
+ continue;
+ }
+ if (auto strTy = mlir::dyn_cast<fir::RecordType>(type)) {
+ auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(*it);
+ if (!intAttr) {
+ mlir::emitError(coor.getLoc(),
+ "expected field name in fir.coordinate_of");
+ return std::nullopt;
+ }
+ type = strTy.getType(intAttr.getInt());
} else if (auto strTy = mlir::dyn_cast<mlir::TupleType>(type)) {
- type = strTy.getType(getConstantIntValue(nxtOpnd));
- } else {
- return true;
+ auto value = llvm::dyn_cast<mlir::Value>(*it);
+ if (!value) {
+ mlir::emitError(
+ coor.getLoc(),
+ "expected constant value to address tuple in fir.coordinate_of");
+ return std::nullopt;
+ }
+ type = strTy.getType(getConstantIntValue(value));
+ } else if (auto charType = mlir::dyn_cast<fir::CharacterType>(type)) {
+ // Addressing character in string. Fortran strings degenerate to arrays
+ // in LLVM, so they are handled like arrays of characters here.
+ if (charType.getLen() == fir::CharacterType::unknownLen())
+ return ShapeAnalysis{false, true};
+ type = fir::CharacterType::getSingleton(charType.getContext(),
+ charType.getFKind());
}
+ ++it;
}
- return true;
+ return ShapeAnalysis{hasKnownShape, columnIsDeferred};
}
private:
@@ -2754,9 +2775,11 @@ struct CoordinateOpConversion
mlir::LLVM::IntegerOverflowFlags nsw =
mlir::LLVM::IntegerOverflowFlags::nsw;
- for (unsigned i = 1, last = operands.size(); i < last; ++i) {
+ int nextIndexValue = 1;
+ fir::CoordinateIndicesAdaptor indices = coor.getIndices();
+ for (auto it = indices.begin(), end = indices.end(); it != end;) {
if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- if (i != 1)
+ if (it != indices.begin())
TODO(loc, "fir.array nested inside other array and/or derived type");
// Applies byte strides from the box. Ignore lower bound from box
// since fir.coordinate_of indexes are zero based. Lowering takes care
@@ -2764,26 +2787,31 @@ struct CoordinateOpConversion
// types and non contiguous arrays.
auto idxTy = lowerTy().indexType();
mlir::Value off = genConstantIndex(loc, idxTy, rewriter, 0);
- for (unsigned index = i, lastIndex = i + arrTy.getDimension();
- index < lastIndex; ++index) {
- mlir::Value stride = getStrideFromBox(loc, boxTyPair, operands[0],
- index - i, rewriter);
+ unsigned arrayDim = arrTy.getDimension();
+ for (unsigned dim = 0; dim < arrayDim && it != end; ++dim, ++it) {
+ mlir::Value stride =
+ getStrideFromBox(loc, boxTyPair, operands[0], dim, rewriter);
auto sc = rewriter.create<mlir::LLVM::MulOp>(
- loc, idxTy, operands[index], stride, nsw);
+ loc, idxTy, operands[nextIndexValue + dim], stride, nsw);
off = rewriter.create<mlir::LLVM::AddOp>(loc, idxTy, sc, off, nsw);
}
+ nextIndexValue += arrayDim;
resultAddr = rewriter.create<mlir::LLVM::GEPOp>(
loc, llvmPtrTy, byteTy, resultAddr,
llvm::ArrayRef<mlir::LLVM::GEPArg>{off});
- i += arrTy.getDimension() - 1;
cpnTy = arrTy.getEleTy();
} else if (auto recTy = mlir::dyn_cast<fir::RecordType>(cpnTy)) {
- mlir::Value nxtOpnd = operands[i];
- cpnTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+ auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(*it);
+ if (!intAttr)
+ return mlir::emitError(loc,
+ "expected field name in fir.coordinate_of");
+ int fieldIndex = intAttr.getInt();
+ ++it;
+ cpnTy = recTy.getType(fieldIndex);
auto llvmRecTy = lowerTy().convertType(recTy);
resultAddr = rewriter.create<mlir::LLVM::GEPOp>(
loc, llvmPtrTy, llvmRecTy, resultAddr,
- llvm::ArrayRef<mlir::LLVM::GEPArg>{0, nxtOpnd});
+ llvm::ArrayRef<mlir::LLVM::GEPArg>{0, fieldIndex});
} else {
fir::emitFatalError(loc, "unexpected type in coordinate_of");
}
@@ -2801,92 +2829,71 @@ struct CoordinateOpConversion
// Component Type
mlir::Type cpnTy = fir::dyn_cast_ptrOrBoxEleTy(baseObjectTy);
- bool hasSubdimension = hasSubDimensions(cpnTy);
- bool columnIsDeferred = !hasSubdimension;
-
- if (!supportedCoordinate(cpnTy, operands.drop_front(1)))
- TODO(loc, "unsupported combination of coordinate operands");
-
- const bool hasKnownShape =
- arraysHaveKnownShape(cpnTy, operands.drop_front(1));
-
- // If only the column is `?`, then we can simply place the column value in
- // the 0-th GEP position.
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- if (!hasKnownShape) {
- const unsigned sz = arrTy.getDimension();
- if (arraysHaveKnownShape(arrTy.getEleTy(),
- operands.drop_front(1 + sz))) {
- fir::SequenceType::ShapeRef shape = arrTy.getShape();
- bool allConst = true;
- for (unsigned i = 0; i < sz - 1; ++i) {
- if (shape[i] < 0) {
- allConst = false;
- break;
- }
- }
- if (allConst)
- columnIsDeferred = true;
- }
- }
- }
+
+ const std::optional<ShapeAnalysis> shapeAnalysis =
+ arraysHaveKnownShape(cpnTy, coor);
+ if (!shapeAnalysis)
+ return mlir::failure();
if (fir::hasDynamicSize(fir::unwrapSequenceType(cpnTy)))
return mlir::emitError(
loc, "fir.coordinate_of with a dynamic element size is unsupported");
- if (hasKnownShape || columnIsDeferred) {
+ if (shapeAnalysis->hasKnownShape || shapeAnalysis->columnIsDeferred) {
llvm::SmallVector<mlir::LLVM::GEPArg> offs;
- if (hasKnownShape && hasSubdimension) {
+ if (shapeAnalysis->hasKnownShape) {
offs.push_back(0);
}
+ // Else, only the column is `?` and we can simply place the column value
+ // in the 0-th GEP position.
+
std::optional<int> dims;
llvm::SmallVector<mlir::Value> arrIdx;
- for (std::size_t i = 1, sz = operands.size(); i < sz; ++i) {
- mlir::Value nxtOpnd = operands[i];
-
- if (!cpnTy)
- return mlir::emitError(loc, "invalid coordinate/check failed");
-
- // check if the i-th coordinate relates to an array
- if (dims) {
- arrIdx.push_back(nxtOpnd);
- int dimsLeft = *dims;
- if (dimsLeft > 1) {
- dims = dimsLeft - 1;
- continue;
- }
- cpnTy = mlir::cast<fir::SequenceType>(cpnTy).getElementType();
- // append array range in reverse (FIR arrays are column-major)
- offs.append(arrIdx.rbegin(), arrIdx.rend());
- arrIdx.clear();
- dims.reset();
+ int nextIndexValue = 1;
+ for (auto index : coor.getIndices()) {
+ if (auto intAttr = llvm::dyn_cast<mlir::IntegerAttr>(index)) {
+ // Addressing derived type component.
+ auto recordType = llvm::dyn_cast<fir::RecordType>(cpnTy);
+ if (!recordType)
+ return mlir::emitError(
+ loc,
+ "fir.coordinate base type is not consistent with operands");
+ int fieldId = intAttr.getInt();
+ cpnTy = recordType.getType(fieldId);
+ offs.push_back(fieldId);
continue;
}
- if (auto arrTy = mlir::dyn_cast<fir::SequenceType>(cpnTy)) {
- int d = arrTy.getDimension() - 1;
- if (d > 0) {
- dims = d;
- arrIdx.push_back(nxtOpnd);
- continue;
+ // Value index (addressing array, tuple, or complex part).
+ mlir::Value indexValue = operands[nextIndexValue++];
+ if (auto tupTy = mlir::dyn_cast<mlir::TupleType>(cpnTy)) {
+ cpnTy = tupTy.getType(getConstantIntValue(indexValue));
+ offs.push_back(indexValue);
+ } else {
+ if (!dims) {
+ if (auto arrayType = llvm::dyn_cast<fir::SequenceType>(cpnTy)) {
+ // Starting addressing array or array component.
+ dims = arrayType.getDimension();
+ cpnTy = arrayType.getElementType();
+ }
+ }
+ if (dims) {
+ arrIdx.push_back(indexValue);
+ if (--(*dims) == 0) {
+ // Append array range in reverse (FIR arrays are column-major).
+ offs.append(arrIdx.rbegin(), arrIdx.rend());
+ arrIdx.clear();
+ dims.reset();
+ ...
[truncated]
|
tblah
approved these changes
Feb 27, 2025
|
Thanks for the reviews! |
jeanPerier
added a commit
to jeanPerier/llvm-project
that referenced
this pull request
Feb 28, 2025
After llvm#127231, fir.coordinate_of should directly carry the field. I updated the lowering and codegen tests in 12731, but not the FIR to FIR tests, which is what this patch is cleaning up.
jph-13
pushed a commit
to jph-13/llvm-project
that referenced
this pull request
Feb 28, 2025
This patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands. The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible. This makes IR transformation harder/brittle, so I want to update FIR to get rid if this. Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change.
cheezeburglar
pushed a commit
to cheezeburglar/llvm-project
that referenced
this pull request
Feb 28, 2025
This patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands. The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible. This makes IR transformation harder/brittle, so I want to update FIR to get rid if this. Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 2, 2025
lands and reverts: needs flang team to reland a8db1fb [flang] update fir.coordinate_of to carry the fields (llvm#127231
vzakhari
added a commit
to vzakhari/llvm-project
that referenced
this pull request
Mar 2, 2025
The issue came up after llvm#127231, when fir.coordinate_of, fed into a declare, only has the field attribute and no coordinates.
jeanPerier
added a commit
that referenced
this pull request
Mar 3, 2025
vzakhari
added a commit
that referenced
this pull request
Mar 3, 2025
The issue came up after #127231, when fir.coordinate_of, fed into a declare, only has the field attribute and no coordinates.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 3, 2025
Flang team looking into: land and reverted 3/1/25 a8db1fb [flang] update fir.coordinate_of to carry the fields (llvm#127231 subsequent landed and reverted, relates to prev. 9805854 [flang][NFC] clean-up fir.field_index legacy usages in tests (llvm#129219)
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 3, 2025
This patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands. The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible. This makes IR transformation harder/brittle, so I want to update FIR to get rid if this. Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 3, 2025
…9219) After llvm#127231, fir.coordinate_of should directly carry the field. I updated the lowering and codegen tests in llvm#12731, but not the FIR to FIR tests, which is what this patch is cleaning up.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Mar 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch updates fir.coordinate_op to carry the field index as attributes instead of relying on getting it from the fir.field_index operations defining its operands.
The rational is that FIR currently has a few operations that require DAGs to be preserved in order to be able to do code generation. This is the case of fir.coordinate_op, which requires its fir.field operand producer to be visible.
This make IR transformation harder/brittle, so I want to update FIR to get rid if this.
Codegen/printer/parser of fir.coordinate_of and many tests need to be updated after this change.
After this patch similar changes should be done to make FIR more robust: