[flang-commits] [flang] ca0ed40 - Remove builder that takes SSA value instead of Attribute on ExtractValueOp, InsetValueOp, and InsertOnRangeOp
Mehdi Amini via flang-commits
flang-commits at lists.llvm.org
Tue Nov 2 15:58:25 PDT 2021
Author: Mehdi Amini
Date: 2021-11-02T22:35:47Z
New Revision: ca0ed40e0000833e739f997f320bfafae35b4996
URL: https://github.com/llvm/llvm-project/commit/ca0ed40e0000833e739f997f320bfafae35b4996
DIFF: https://github.com/llvm/llvm-project/commit/ca0ed40e0000833e739f997f320bfafae35b4996.diff
LOG: Remove builder that takes SSA value instead of Attribute on ExtractValueOp, InsetValueOp, and InsertOnRangeOp
This builder exposed a somehow "unsafe" API: it pretends we can
construct an InsertOnRangeOp from a range of SSA values, even though
this will crash if these aren't the result of `arith.constant` since
the operation actually needs attribute values (a build method can't
fail gracefully).
That means that the caller must check for the producer, at which
point they can just assemble the attribute array directly and call
the existing builder.
The existing call-sites were even in a worse state here: they would
actually create a constant operation that wouldn't be used and only
serve to carry the attribute through the builder API.
Differential Revision: https://reviews.llvm.org/D112946
Added:
Modified:
flang/include/flang/Lower/ComplexExpr.h
flang/include/flang/Optimizer/Dialect/FIROps.td
flang/lib/Lower/CharacterExpr.cpp
flang/lib/Optimizer/Builder/Character.cpp
flang/lib/Optimizer/Dialect/FIROps.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Lower/ComplexExpr.h b/flang/include/flang/Lower/ComplexExpr.h
index d3600a0cda6a5..337294587cf56 100644
--- a/flang/include/flang/Lower/ComplexExpr.h
+++ b/flang/include/flang/Lower/ComplexExpr.h
@@ -57,14 +57,18 @@ class ComplexExprHelper {
protected:
template <Part partId>
mlir::Value extract(mlir::Value cplx) {
- return builder.create<fir::ExtractValueOp>(loc, getComplexPartType(cplx),
- cplx, createPartId<partId>());
+ return builder.create<fir::ExtractValueOp>(
+ loc, getComplexPartType(cplx), cplx,
+ builder.getArrayAttr({builder.getIntegerAttr(
+ builder.getIndexType(), static_cast<int>(partId))}));
}
template <Part partId>
mlir::Value insert(mlir::Value cplx, mlir::Value part) {
- return builder.create<fir::InsertValueOp>(loc, cplx.getType(), cplx, part,
- createPartId<partId>());
+ return builder.create<fir::InsertValueOp>(
+ loc, cplx.getType(), cplx, part,
+ builder.getArrayAttr({builder.getIntegerAttr(
+ builder.getIndexType(), static_cast<int>(partId))}));
}
template <Part partId>
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 6a9fbcdf28a4e..963c06de66597 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -1725,11 +1725,6 @@ def fir_ExtractValueOp : fir_OneResultOp<"extract_value", [NoSideEffect]> {
let assemblyFormat = [{
$adt `,` $coor attr-dict `:` functional-type(operands, results)
}];
-
- let builders = [
- OpBuilder<(ins "mlir::Type":$rty, "mlir::Value":$adt,
- "llvm::ArrayRef<mlir::Value>":$vcoor)>
- ];
}
def fir_FieldIndexOp : fir_OneResultOp<"field_index", [NoSideEffect]> {
@@ -1976,11 +1971,6 @@ def fir_InsertValueOp : fir_OneResultOp<"insert_value", [NoSideEffect]> {
$adt `,` $val `,` $coor attr-dict `:` functional-type(operands, results)
}];
- let builders = [
- OpBuilder<(ins "mlir::Type":$rty, "mlir::Value":$adt, "mlir::Value":$val,
- "llvm::ArrayRef<mlir::Value>":$vcoor)>
- ];
-
let hasCanonicalizer = 1;
}
@@ -2011,11 +2001,6 @@ def fir_InsertOnRangeOp : fir_OneResultOp<"insert_on_range", [NoSideEffect]> {
$seq `,` $val `,` $coor attr-dict `:` functional-type(operands, results)
}];
- let builders = [
- OpBuilder<(ins "mlir::Type":$rty, "mlir::Value":$adt, "mlir::Value":$val,
- "llvm::ArrayRef<mlir::Value>":$vcoor)>
- ];
-
let verifier = "return ::verify(*this);";
}
diff --git a/flang/lib/Lower/CharacterExpr.cpp b/flang/lib/Lower/CharacterExpr.cpp
index 551051d7987d4..0974d2f40d702 100644
--- a/flang/lib/Lower/CharacterExpr.cpp
+++ b/flang/lib/Lower/CharacterExpr.cpp
@@ -217,9 +217,10 @@ void Fortran::lower::CharacterExprHelper::createLengthOneAssign(
auto valTy = val.getType();
// Precondition is rhs is size 1, but it may be wrapped in a fir.array.
if (auto seqTy = valTy.dyn_cast<fir::SequenceType>()) {
- auto zero = builder.createIntegerConstant(loc, builder.getIndexType(), 0);
+ auto zero = builder.getIntegerAttr(builder.getIndexType(), 0);
valTy = seqTy.getEleTy();
- val = builder.create<fir::ExtractValueOp>(loc, valTy, val, zero);
+ val = builder.create<fir::ExtractValueOp>(loc, valTy, val,
+ builder.getArrayAttr(zero));
}
auto addrTy = fir::ReferenceType::get(valTy);
addr = builder.createConvert(loc, addrTy, addr);
diff --git a/flang/lib/Optimizer/Builder/Character.cpp b/flang/lib/Optimizer/Builder/Character.cpp
index 7cd2b11f8cc4f..b306d54313006 100644
--- a/flang/lib/Optimizer/Builder/Character.cpp
+++ b/flang/lib/Optimizer/Builder/Character.cpp
@@ -680,8 +680,9 @@ fir::factory::CharacterExprHelper::createSingletonFromCode(mlir::Value code,
auto intType = builder.getIntegerType(bits);
auto cast = builder.createConvert(loc, intType, code);
auto undef = builder.create<fir::UndefOp>(loc, charType);
- auto zero = builder.createIntegerConstant(loc, builder.getIndexType(), 0);
- return builder.create<fir::InsertValueOp>(loc, charType, undef, cast, zero);
+ auto zero = builder.getIntegerAttr(builder.getIndexType(), 0);
+ return builder.create<fir::InsertValueOp>(loc, charType, undef, cast,
+ builder.getArrayAttr(zero));
}
mlir::Value fir::factory::CharacterExprHelper::extractCodeFromSingleton(
@@ -690,8 +691,9 @@ mlir::Value fir::factory::CharacterExprHelper::extractCodeFromSingleton(
assert(type.getLen() == 1);
auto bits = builder.getKindMap().getCharacterBitsize(type.getFKind());
auto intType = builder.getIntegerType(bits);
- auto zero = builder.createIntegerConstant(loc, builder.getIndexType(), 0);
- return builder.create<fir::ExtractValueOp>(loc, intType, singleton, zero);
+ auto zero = builder.getIntegerAttr(builder.getIndexType(), 0);
+ return builder.create<fir::ExtractValueOp>(loc, intType, singleton,
+ builder.getArrayAttr(zero));
}
mlir::Value
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 83494d18aef5b..6196d3a375b72 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -1285,39 +1285,6 @@ mlir::ParseResult fir::GlobalOp::verifyValidLinkage(StringRef linkage) {
return mlir::success(llvm::is_contained(validNames, linkage));
}
-template <bool AllowFields>
-static void appendAsAttribute(llvm::SmallVectorImpl<mlir::Attribute> &attrs,
- mlir::Value val) {
- if (auto *op = val.getDefiningOp()) {
- if (auto cop = mlir::dyn_cast<mlir::arith::ConstantOp>(op)) {
- // append the integer constant value
- if (auto iattr = cop.value().dyn_cast<mlir::IntegerAttr>()) {
- attrs.push_back(iattr);
- return;
- }
- } else if (auto fld = mlir::dyn_cast<fir::FieldIndexOp>(op)) {
- if constexpr (AllowFields) {
- // append the field name and the record type
- attrs.push_back(fld.field_idAttr());
- attrs.push_back(fld.on_typeAttr());
- return;
- }
- }
- }
- llvm::report_fatal_error("cannot build Op with these arguments");
-}
-
-template <bool AllowFields = true>
-static mlir::ArrayAttr collectAsAttributes(mlir::MLIRContext *ctxt,
- OperationState &result,
- llvm::ArrayRef<mlir::Value> inds) {
- llvm::SmallVector<mlir::Attribute> attrs;
- for (auto v : inds)
- appendAsAttribute<AllowFields>(attrs, v);
- assert(!attrs.empty());
- return mlir::ArrayAttr::get(ctxt, attrs);
-}
-
//===----------------------------------------------------------------------===//
// GlobalLenOp
//===----------------------------------------------------------------------===//
@@ -1351,18 +1318,6 @@ static void print(mlir::OpAsmPrinter &p, fir::GlobalLenOp &op) {
<< ", " << op.getOperation()->getAttr(fir::GlobalLenOp::intAttrName());
}
-//===----------------------------------------------------------------------===//
-// ExtractValueOp
-//===----------------------------------------------------------------------===//
-
-void fir::ExtractValueOp::build(mlir::OpBuilder &builder,
- OperationState &result, mlir::Type resTy,
- mlir::Value aggVal,
- llvm::ArrayRef<mlir::Value> inds) {
- auto aa = collectAsAttributes<>(builder.getContext(), result, inds);
- build(builder, result, resTy, aggVal, aa);
-}
-
//===----------------------------------------------------------------------===//
// FieldIndexOp
//===----------------------------------------------------------------------===//
@@ -1430,14 +1385,6 @@ void fir::FieldIndexOp::build(mlir::OpBuilder &builder,
// InsertOnRangeOp
//===----------------------------------------------------------------------===//
-void fir::InsertOnRangeOp::build(mlir::OpBuilder &builder,
- OperationState &result, mlir::Type resTy,
- mlir::Value aggVal, mlir::Value eleVal,
- llvm::ArrayRef<mlir::Value> inds) {
- auto aa = collectAsAttributes<false>(builder.getContext(), result, inds);
- build(builder, result, resTy, aggVal, eleVal, aa);
-}
-
/// Range bounds must be nonnegative, and the range must not be empty.
static mlir::LogicalResult verify(fir::InsertOnRangeOp op) {
if (op.coor().size() < 2 || op.coor().size() % 2 != 0)
@@ -1461,14 +1408,6 @@ static mlir::LogicalResult verify(fir::InsertOnRangeOp op) {
// InsertValueOp
//===----------------------------------------------------------------------===//
-void fir::InsertValueOp::build(mlir::OpBuilder &builder, OperationState &result,
- mlir::Type resTy, mlir::Value aggVal,
- mlir::Value eleVal,
- llvm::ArrayRef<mlir::Value> inds) {
- auto aa = collectAsAttributes<>(builder.getContext(), result, inds);
- build(builder, result, resTy, aggVal, eleVal, aa);
-}
-
static bool checkIsIntegerConstant(mlir::Attribute attr, int64_t conVal) {
if (auto iattr = attr.dyn_cast<mlir::IntegerAttr>())
return iattr.getInt() == conVal;
More information about the flang-commits
mailing list