Skip to content

Commit 56f62fb

Browse files
committed
[mlir] Finish removing Identifier from the C++ API
There have been a few API pieces remaining to allow for a smooth transition for downstream users, but these have been up for a few months now. After this only the C API will have reference to "Identifier", but those will be reworked in a followup. The main updates are: * Identifier -> StringAttr * StringAttr::get requires the context as the first parameter - i.e. `Identifier::get("...", ctx)` -> `StringAttr::get(ctx, "...")` Reviewed By: mehdi_amini Differential Revision: https://reviews.llvm.org/D116626
1 parent be1aeb8 commit 56f62fb

File tree

13 files changed

+16
-52
lines changed

13 files changed

+16
-52
lines changed

cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
#include "mlir/IR/BuiltinAttributes.h"
22
#include "mlir/IR/BuiltinTypes.h"
3-
#include "mlir/IR/Identifier.h"
43
#include "mlir/IR/Location.h"
54
#include "mlir/IR/MLIRContext.h"
65
#include "mlir/IR/OperationSupport.h"
76

87
mlir::MLIRContext Context;
98

10-
auto Identifier = mlir::Identifier::get("foo", &Context);
9+
auto Identifier = mlir::StringAttr::get(&Context, "foo");
1110
mlir::OperationName OperationName("FooOp", &Context);
1211

1312
mlir::Type Type(nullptr);

flang/lib/Lower/FIRBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ mlir::Value Fortran::lower::FirOpBuilder::allocateLocal(
8181
});
8282
llvm::SmallVector<mlir::NamedAttribute, 2> attrs;
8383
if (asTarget)
84-
attrs.emplace_back(mlir::Identifier::get("target", getContext()),
84+
attrs.emplace_back(mlir::StringAttr::get(getContext(), "target"),
8585
getUnitAttr());
8686
return create<fir::AllocaOp>(loc, ty, nm, llvm::None, indices, attrs);
8787
}
@@ -175,9 +175,9 @@ mlir::Value Fortran::lower::FirOpBuilder::createConvert(mlir::Location loc,
175175
fir::StringLitOp Fortran::lower::FirOpBuilder::createStringLit(
176176
mlir::Location loc, mlir::Type eleTy, llvm::StringRef data) {
177177
auto strAttr = mlir::StringAttr::get(getContext(), data);
178-
auto valTag = mlir::Identifier::get(fir::StringLitOp::value(), getContext());
178+
auto valTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::value());
179179
mlir::NamedAttribute dataAttr(valTag, strAttr);
180-
auto sizeTag = mlir::Identifier::get(fir::StringLitOp::size(), getContext());
180+
auto sizeTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::size());
181181
mlir::NamedAttribute sizeAttr(sizeTag, getI64IntegerAttr(data.size()));
182182
llvm::SmallVector<mlir::NamedAttribute, 2> attrs{dataAttr, sizeAttr};
183183
auto arrTy =

flang/lib/Optimizer/Builder/FIRBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ mlir::Value fir::FirOpBuilder::allocateLocal(
162162
llvm::SmallVector<mlir::NamedAttribute> attrs;
163163
if (asTarget)
164164
attrs.emplace_back(
165-
mlir::Identifier::get(fir::getTargetAttrName(), getContext()),
165+
mlir::StringAttr::get(getContext(), fir::getTargetAttrName()),
166166
getUnitAttr());
167167
// Create the local variable.
168168
if (name.empty()) {
@@ -298,9 +298,9 @@ fir::StringLitOp fir::FirOpBuilder::createStringLitOp(mlir::Location loc,
298298
llvm::StringRef data) {
299299
auto type = fir::CharacterType::get(getContext(), 1, data.size());
300300
auto strAttr = mlir::StringAttr::get(getContext(), data);
301-
auto valTag = mlir::Identifier::get(fir::StringLitOp::value(), getContext());
301+
auto valTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::value());
302302
mlir::NamedAttribute dataAttr(valTag, strAttr);
303-
auto sizeTag = mlir::Identifier::get(fir::StringLitOp::size(), getContext());
303+
auto sizeTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::size());
304304
mlir::NamedAttribute sizeAttr(sizeTag, getI64IntegerAttr(data.size()));
305305
llvm::SmallVector<mlir::NamedAttribute> attrs{dataAttr, sizeAttr};
306306
return create<fir::StringLitOp>(loc, llvm::ArrayRef<mlir::Type>{type},

flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ TEST_F(FIRBuilderTest, uniqueCFIdent) {
238238

239239
TEST_F(FIRBuilderTest, locationToLineNo) {
240240
auto builder = getBuilder();
241-
auto loc = mlir::FileLineColLoc::get(builder.getIdentifier("file1"), 10, 5);
241+
auto loc = mlir::FileLineColLoc::get(builder.getStringAttr("file1"), 10, 5);
242242
mlir::Value line =
243243
fir::factory::locationToLineNo(builder, loc, builder.getI64Type());
244244
checkIntegerConstant(line, builder.getI64Type(), 10);
@@ -260,7 +260,7 @@ TEST_F(FIRBuilderTest, hasDynamicSize) {
260260
TEST_F(FIRBuilderTest, locationToFilename) {
261261
auto builder = getBuilder();
262262
auto loc =
263-
mlir::FileLineColLoc::get(builder.getIdentifier("file1.f90"), 10, 5);
263+
mlir::FileLineColLoc::get(builder.getStringAttr("file1.f90"), 10, 5);
264264
mlir::Value locToFile = fir::factory::locationToFilename(builder, loc);
265265
auto addrOp = dyn_cast<fir::AddrOfOp>(locToFile.getDefiningOp());
266266
auto symbol = addrOp.symbol().getRootReference().getValue();

mlir/include/mlir/IR/Attributes.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
namespace mlir {
1616
class StringAttr;
1717

18-
// TODO: Remove this when all usages have been replaced with StringAttr.
19-
using Identifier = StringAttr;
20-
2118
/// Attributes are known-constant values of operations.
2219
///
2320
/// Instances of the Attribute class are references to immortal key-value pairs

mlir/include/mlir/IR/Builders.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ class Builder {
5353

5454
MLIRContext *getContext() const { return context; }
5555

56-
StringAttr getIdentifier(const Twine &str);
57-
5856
// Locations.
5957
Location getUnknownLoc();
6058
Location getFusedLoc(ArrayRef<Location> locs,

mlir/include/mlir/IR/BuiltinAttributes.td

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -949,12 +949,6 @@ def Builtin_StringAttr : Builtin_Attr<"String"> {
949949
return getValue().compare(rhs.getValue());
950950
}
951951

952-
/// FIXME: Defined as part of transition of Identifier->StringAttr. Prefer
953-
/// using the other `get` methods instead.
954-
static StringAttr get(const Twine &str, MLIRContext *context) {
955-
return get(context, str);
956-
}
957-
958952
private:
959953
/// Return an empty StringAttr with NoneType type. This is a special variant
960954
/// of the `get` method that is used by the MLIRContext to cache the

mlir/include/mlir/IR/Identifier.h

Lines changed: 0 additions & 20 deletions
This file was deleted.

mlir/lib/CAPI/IR/IR.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -820,8 +820,8 @@ MlirLogicalResult mlirSymbolTableReplaceAllSymbolUses(MlirStringRef oldSymbol,
820820
MlirOperation from) {
821821
auto *cppFrom = unwrap(from);
822822
auto *context = cppFrom->getContext();
823-
auto oldSymbolAttr = StringAttr::get(unwrap(oldSymbol), context);
824-
auto newSymbolAttr = StringAttr::get(unwrap(newSymbol), context);
823+
auto oldSymbolAttr = StringAttr::get(context, unwrap(oldSymbol));
824+
auto newSymbolAttr = StringAttr::get(context, unwrap(newSymbol));
825825
return wrap(SymbolTable::replaceAllSymbolUses(oldSymbolAttr, newSymbolAttr,
826826
unwrap(from)));
827827
}

mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,12 +1182,12 @@ static void populateVectorizationPatterns(
11821182
ConvOp::getOperationName(), context,
11831183
LinalgTilingOptions().setTileSizes(tileSizes),
11841184
LinalgTransformationFilter(ArrayRef<StringAttr>{},
1185-
StringAttr::get(kTiledMarker, context)));
1185+
StringAttr::get(context, kTiledMarker)));
11861186

11871187
promotionPatterns.add<LinalgPromotionPattern<ConvOp>>(
11881188
context, LinalgPromotionOptions().setUseFullTileBuffersByDefault(true),
1189-
LinalgTransformationFilter(StringAttr::get(kTiledMarker, context),
1190-
StringAttr::get(kPromotedMarker, context)));
1189+
LinalgTransformationFilter(StringAttr::get(context, kTiledMarker),
1190+
StringAttr::get(context, kPromotedMarker)));
11911191

11921192
SmallVector<bool, 4> mask(N);
11931193
int offset = tileSizes.size() - N;

mlir/lib/IR/Builders.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@
1919

2020
using namespace mlir;
2121

22-
StringAttr Builder::getIdentifier(const Twine &str) {
23-
return getStringAttr(str);
24-
}
25-
2622
//===----------------------------------------------------------------------===//
2723
// Locations.
2824
//===----------------------------------------------------------------------===//

mlir/lib/Pass/Pass.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ struct OpPassManagerImpl {
109109
/// Return the operation name of this pass manager as an identifier.
110110
StringAttr getOpName(MLIRContext &context) {
111111
if (!identifier)
112-
identifier = StringAttr::get(name, &context);
112+
identifier = StringAttr::get(&context, name);
113113
return *identifier;
114114
}
115115

mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ void TestLinalgDistribution::runOnFunction() {
5959
distributeTiledLoopsPatterns, getDistributionOptions(),
6060
LinalgTransformationFilter(
6161
ArrayRef<StringAttr>{},
62-
{StringAttr::get("distributed", funcOp.getContext())})
62+
{StringAttr::get(funcOp.getContext(), "distributed")})
6363
.addFilter([](Operation *op) {
6464
return success(!op->getParentOfType<linalg::TiledLoopOp>());
6565
}));

0 commit comments

Comments
 (0)