[mlir] Finish removing Identifier from the C++ API

[rriddle]

Summary

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, "...")

[jpienaar]

Is the changes needed downstream effectively Identifier::get -> StringAttr::get with args switched? (and header removed)

[rriddle]

In D116626#3220888, @jpienaar wrote:

Is the changes needed downstream effectively Identifier::get -> StringAttr::get with args switched? (and header removed)

Essentially yeah, given that Identifier is already removed. This is just removing the placeholder APIs(names) that were used for staging.

[mehdi_amini]

LGTM, please include in the description of the commit that the fix is to reverse the argument order.

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

 #include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/BuiltinTypes.h"
-#include "mlir/IR/Identifier.h"
 #include "mlir/IR/Location.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/IR/OperationSupport.h"
 
 mlir::MLIRContext Context;
 
-auto Identifier = mlir::Identifier::get("foo", &Context);
+auto Identifier = mlir::StringAttr::get(&Context, "foo");
 mlir::OperationName OperationName("FooOp", &Context);
 
 mlir::Type Type(nullptr);
 mlir::Type IndexType = mlir::IndexType::get(&Context);
 mlir::Type IntegerType =
     mlir::IntegerType::get(&Context, 3, mlir::IntegerType::Unsigned);
 mlir::Type FloatType = mlir::Float32Type::get(&Context);
 mlir::Type MemRefType = mlir::MemRefType::get({4, 5}, FloatType);

flang/lib/Lower/FIRBuilder.cpp

mlir::Value Fortran::lower::FirOpBuilder::allocateLocal(
     llvm::ArrayRef<mlir::Value> shape, bool asTarget) {
   llvm::SmallVector<mlir::Value, 8> indices;
   auto idxTy = getIndexType();
   llvm::for_each(shape, [&](mlir::Value sh) {
     indices.push_back(createConvert(loc, idxTy, sh));
   });
   llvm::SmallVector<mlir::NamedAttribute, 2> attrs;
   if (asTarget)
-    attrs.emplace_back(mlir::Identifier::get("target", getContext()),
+    attrs.emplace_back(mlir::StringAttr::get(getContext(), "target"),
                        getUnitAttr());
   return create<fir::AllocaOp>(loc, ty, nm, llvm::None, indices, attrs);
 }
 
 /// Create a temporary variable on the stack. Anonymous temporaries have no
 /// `name` value.
 mlir::Value Fortran::lower::FirOpBuilder::createTemporary(
     mlir::Location loc, mlir::Type type, llvm::StringRef name,
mlir::Value Fortran::lower::FirOpBuilder::createConvert(mlir::Location loc,
   if (val.getType() != toTy)
     return create<fir::ConvertOp>(loc, toTy, val);
   return val;
 }
 
 fir::StringLitOp Fortran::lower::FirOpBuilder::createStringLit(
     mlir::Location loc, mlir::Type eleTy, llvm::StringRef data) {
   auto strAttr = mlir::StringAttr::get(getContext(), data);
-  auto valTag = mlir::Identifier::get(fir::StringLitOp::value(), getContext());
+  auto valTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::value());
   mlir::NamedAttribute dataAttr(valTag, strAttr);
-  auto sizeTag = mlir::Identifier::get(fir::StringLitOp::size(), getContext());
+  auto sizeTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::size());
   mlir::NamedAttribute sizeAttr(sizeTag, getI64IntegerAttr(data.size()));
   llvm::SmallVector<mlir::NamedAttribute, 2> attrs{dataAttr, sizeAttr};
   auto arrTy =
       fir::SequenceType::get(fir::SequenceType::Shape(1, data.size()), eleTy);
   return create<fir::StringLitOp>(loc, llvm::ArrayRef<mlir::Type>{arrTy},
                                   llvm::None, attrs);
 }

flang/lib/Optimizer/Builder/FIRBuilder.cpp

mlir::Value fir::FirOpBuilder::allocateLocal(
   auto idxTy = getIndexType();
   llvm::for_each(elidedShape, [&](mlir::Value sh) {
     indices.push_back(createConvert(loc, idxTy, sh));
   });
   // Add a target attribute, if needed.
   llvm::SmallVector<mlir::NamedAttribute> attrs;
   if (asTarget)
     attrs.emplace_back(
-        mlir::Identifier::get(fir::getTargetAttrName(), getContext()),
+        mlir::StringAttr::get(getContext(), fir::getTargetAttrName()),
         getUnitAttr());
   // Create the local variable.
   if (name.empty()) {
     if (uniqName.empty())
       return create<fir::AllocaOp>(loc, ty, pinned, elidedLenParams, indices,
                                    attrs);
     return create<fir::AllocaOp>(loc, ty, uniqName, pinned, elidedLenParams,
                                  indices, attrs);
mlir::Value fir::FirOpBuilder::createConvert(mlir::Location loc,
   }
   return val;
 }
 
 fir::StringLitOp fir::FirOpBuilder::createStringLitOp(mlir::Location loc,
                                                       llvm::StringRef data) {
   auto type = fir::CharacterType::get(getContext(), 1, data.size());
   auto strAttr = mlir::StringAttr::get(getContext(), data);
-  auto valTag = mlir::Identifier::get(fir::StringLitOp::value(), getContext());
+  auto valTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::value());
   mlir::NamedAttribute dataAttr(valTag, strAttr);
-  auto sizeTag = mlir::Identifier::get(fir::StringLitOp::size(), getContext());
+  auto sizeTag = mlir::StringAttr::get(getContext(), fir::StringLitOp::size());
   mlir::NamedAttribute sizeAttr(sizeTag, getI64IntegerAttr(data.size()));
   llvm::SmallVector<mlir::NamedAttribute> attrs{dataAttr, sizeAttr};
   return create<fir::StringLitOp>(loc, llvm::ArrayRef<mlir::Type>{type},
                                   llvm::None, attrs);
 }
 
 mlir::Value fir::FirOpBuilder::genShape(mlir::Location loc,
                                         llvm::ArrayRef<mlir::Value> exts) {

flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp

TEST_F(FIRBuilderTest, uniqueCFIdent) {
   EXPECT_EQ("_QQ.c22a886b2f30ea8c064ef1178377fc31", str1);
   str1 = fir::factory::uniqueCGIdent(
       "pr", "longnamemorethan32characterneedshashing");
   EXPECT_EQ("_QQpr.c22a886b2f30ea8c064ef1178377fc31", str1);
 }
 
 TEST_F(FIRBuilderTest, locationToLineNo) {
   auto builder = getBuilder();
-  auto loc = mlir::FileLineColLoc::get(builder.getIdentifier("file1"), 10, 5);
+  auto loc = mlir::FileLineColLoc::get(builder.getStringAttr("file1"), 10, 5);
   mlir::Value line =
       fir::factory::locationToLineNo(builder, loc, builder.getI64Type());
   checkIntegerConstant(line, builder.getI64Type(), 10);
   line = fir::factory::locationToLineNo(
       builder, builder.getUnknownLoc(), builder.getI64Type());
   checkIntegerConstant(line, builder.getI64Type(), 0);
 }
 
 TEST_F(FIRBuilderTest, hasDynamicSize) {
   auto builder = getBuilder();
   auto type = fir::CharacterType::get(builder.getContext(), 1, 16);
   EXPECT_FALSE(fir::hasDynamicSize(type));
   EXPECT_TRUE(fir::SequenceType::getUnknownExtent());
   auto seqTy = builder.getVarLenSeqTy(builder.getI64Type(), 10);
   EXPECT_TRUE(fir::hasDynamicSize(seqTy));
   EXPECT_FALSE(fir::hasDynamicSize(builder.getI64Type()));
 }
 
 TEST_F(FIRBuilderTest, locationToFilename) {
   auto builder = getBuilder();
   auto loc =
-      mlir::FileLineColLoc::get(builder.getIdentifier("file1.f90"), 10, 5);
+      mlir::FileLineColLoc::get(builder.getStringAttr("file1.f90"), 10, 5);
   mlir::Value locToFile = fir::factory::locationToFilename(builder, loc);
   auto addrOp = dyn_cast<fir::AddrOfOp>(locToFile.getDefiningOp());
   auto symbol = addrOp.symbol().getRootReference().getValue();
   auto global = builder.getNamedGlobal(symbol);
   auto stringLitOps = global.getRegion().front().getOps<fir::StringLitOp>();
   EXPECT_TRUE(llvm::hasSingleElement(stringLitOps));
   for (auto stringLit : stringLitOps) {
     EXPECT_EQ(10, stringLit.getSize().cast<mlir::IntegerAttr>().getValue());

mlir/include/mlir/IR/Attributes.h

 #define MLIR_IR_ATTRIBUTES_H
 
 #include "mlir/IR/AttributeSupport.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 
 namespace mlir {
 class StringAttr;
 
-// TODO: Remove this when all usages have been replaced with StringAttr.
-using Identifier = StringAttr;
-
 /// Attributes are known-constant values of operations.
 ///
 /// Instances of the Attribute class are references to immortal key-value pairs
 /// with immutable, uniqued keys owned by MLIRContext. As such, an Attribute is
 /// a thin wrapper around an underlying storage pointer. Attributes are usually
 /// passed by value.
 class Attribute {
 public:

mlir/include/mlir/IR/Builders.h

 /// like types, attributes, and affine expressions.
 class Builder {
 public:
   explicit Builder(MLIRContext *context) : context(context) {}
   explicit Builder(Operation *op) : Builder(op->getContext()) {}
 
   MLIRContext *getContext() const { return context; }
 
-  StringAttr getIdentifier(const Twine &str);
-
   // Locations.
   Location getUnknownLoc();
   Location getFusedLoc(ArrayRef<Location> locs,
                        Attribute metadata = Attribute());
 
   // Types.
   FloatType getBF16Type();
   FloatType getF16Type();

mlir/include/mlir/IR/BuiltinAttributes.td

let extraClassDeclaration = [{
 
     /// Compare the underlying string value to the one in `rhs`.
     int compare(StringAttr rhs) const {
       if (*this == rhs)
         return 0;
       return getValue().compare(rhs.getValue());
     }
 
-    /// FIXME: Defined as part of transition of Identifier->StringAttr. Prefer
-    /// using the other `get` methods instead.
-    static StringAttr get(const Twine &str, MLIRContext *context) {
-      return get(context, str);
-    }
-
   private:
     /// Return an empty StringAttr with NoneType type. This is a special variant
     /// of the `get` method that is used by the MLIRContext to cache the
     /// instance.
     static StringAttr getEmptyStringAttrUnchecked(MLIRContext *context);
     friend MLIRContext;
   public:
   }];

mlir/include/mlir/IR/Identifier.h

-//===- Identifier.h - MLIR Identifier Class ---------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef MLIR_IR_IDENTIFIER_H
-#define MLIR_IR_IDENTIFIER_H
-
-#include "mlir/IR/BuiltinAttributes.h"
-
-namespace mlir {
-/// NOTICE: Identifier is deprecated and usages of it should be replaced with
-/// StringAttr.
-using Identifier = StringAttr;
-} // namespace mlir
-
-#endif

mlir/lib/CAPI/IR/IR.cpp

void mlirSymbolTableErase(MlirSymbolTable symbolTable,
   unwrap(symbolTable)->erase(unwrap(operation));
 }
 
 MlirLogicalResult mlirSymbolTableReplaceAllSymbolUses(MlirStringRef oldSymbol,
                                                       MlirStringRef newSymbol,
                                                       MlirOperation from) {
   auto *cppFrom = unwrap(from);
   auto *context = cppFrom->getContext();
-  auto oldSymbolAttr = StringAttr::get(unwrap(oldSymbol), context);
-  auto newSymbolAttr = StringAttr::get(unwrap(newSymbol), context);
+  auto oldSymbolAttr = StringAttr::get(context, unwrap(oldSymbol));
+  auto newSymbolAttr = StringAttr::get(context, unwrap(newSymbol));
   return wrap(SymbolTable::replaceAllSymbolUses(oldSymbolAttr, newSymbolAttr,
                                                 unwrap(from)));
 }
 
 void mlirSymbolTableWalkSymbolTables(MlirOperation from, bool allSymUsesVisible,
                                      void (*callback)(MlirOperation, bool,
                                                       void *userData),
                                      void *userData) {
   SymbolTable::walkSymbolTables(unwrap(from), allSymUsesVisible,
                                 [&](Operation *foundOpCpp, bool isVisible) {
                                   callback(wrap(foundOpCpp), isVisible,
                                            userData);
                                 });
 }

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

static void populateVectorizationPatterns(
   if (tileSizes.size() < N)
     return;
 
   constexpr static StringRef kTiledMarker = "TILED";
   constexpr static StringRef kPromotedMarker = "PROMOTED";
   tilingPatterns.add<LinalgTilingPattern<ConvOp>>(
       context, LinalgTilingOptions().setTileSizes(tileSizes),
       LinalgTransformationFilter(ArrayRef<StringAttr>{},
-                                 StringAttr::get(kTiledMarker, context)));
+                                 StringAttr::get(context, kTiledMarker)));
 
   promotionPatterns.add<LinalgPromotionPattern<ConvOp>>(
       context, LinalgPromotionOptions().setUseFullTileBuffersByDefault(true),
-      LinalgTransformationFilter(StringAttr::get(kTiledMarker, context),
-                                 StringAttr::get(kPromotedMarker, context)));
+      LinalgTransformationFilter(StringAttr::get(context, kTiledMarker),
+                                 StringAttr::get(context, kPromotedMarker)));
 
   SmallVector<bool, 4> mask(N);
   int offset = tileSizes.size() - N;
   std::transform(tileSizes.begin() + offset, tileSizes.end(), mask.begin(),
                  [](int64_t i) -> bool { return i > 1; });
 
   vectorizationPatterns.add<ConvOpVectorization<ConvOp, N>>(context, mask);
 }

mlir/lib/IR/Builders.cpp

 #include "mlir/IR/Dialect.h"
 #include "mlir/IR/IntegerSet.h"
 #include "mlir/IR/Matchers.h"
 #include "mlir/IR/SymbolTable.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace mlir;
 
-StringAttr Builder::getIdentifier(const Twine &str) {
-  return getStringAttr(str);
-}
-
 //===----------------------------------------------------------------------===//
 // Locations.
 //===----------------------------------------------------------------------===//
 
 Location Builder::getUnknownLoc() { return UnknownLoc::get(context); }
 
 Location Builder::getFusedLoc(ArrayRef<Location> locs, Attribute metadata) {
   return FusedLoc::get(locs, metadata, context);

mlir/lib/Pass/Pass.cpp

struct OpPassManagerImpl {
 
   /// Coalesce adjacent AdaptorPasses into one large adaptor. This runs
   /// recursively through the pipeline graph.
   void coalesceAdjacentAdaptorPasses();
 
   /// Return the operation name of this pass manager as an identifier.
   StringAttr getOpName(MLIRContext &context) {
     if (!identifier)
-      identifier = StringAttr::get(name, &context);
+      identifier = StringAttr::get(&context, name);
     return *identifier;
   }
 
   /// The name of the operation that passes of this pass manager operate on.
   std::string name;
 
   /// The cached identifier (internalized in the context) for the name of the
   /// operation that passes of this pass manager operate on.

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

 
 void TestLinalgDistribution::runOnFunction() {
   auto funcOp = getFunction();
   OwningRewritePatternList distributeTiledLoopsPatterns(&getContext());
   populateLinalgDistributeTiledLoopPattern(
       distributeTiledLoopsPatterns, getDistributionOptions(),
       LinalgTransformationFilter(
           ArrayRef<StringAttr>{},
-          {StringAttr::get("distributed", funcOp.getContext())})
+          {StringAttr::get(funcOp.getContext(), "distributed")})
           .addFilter([](Operation *op) {
             return success(!op->getParentOfType<linalg::TiledLoopOp>());
           }));
   (void)applyPatternsAndFoldGreedily(funcOp,
                                      std::move(distributeTiledLoopsPatterns));
   // Ensure we drop the marker in the end.
   funcOp.walk([](LinalgOp op) {
     op->removeAttr(LinalgTransforms::kLinalgTransformMarker);