[flang-commits] [flang] 56f62fb - [mlir] Finish removing Identifier from the C++ API

River Riddle via flang-commits flang-commits at lists.llvm.org
Wed Jan 12 11:58:55 PST 2022


Author: River Riddle
Date: 2022-01-12T11:58:23-08:00
New Revision: 56f62fbf73a2c0844d7f26ae10f2757aac981a2d

URL: https://github.com/llvm/llvm-project/commit/56f62fbf73a2c0844d7f26ae10f2757aac981a2d
DIFF: https://github.com/llvm/llvm-project/commit/56f62fbf73a2c0844d7f26ae10f2757aac981a2d.diff

LOG: [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

Added: 
    

Modified: 
    cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp
    flang/lib/Lower/FIRBuilder.cpp
    flang/lib/Optimizer/Builder/FIRBuilder.cpp
    flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
    mlir/include/mlir/IR/Attributes.h
    mlir/include/mlir/IR/Builders.h
    mlir/include/mlir/IR/BuiltinAttributes.td
    mlir/lib/CAPI/IR/IR.cpp
    mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
    mlir/lib/IR/Builders.cpp
    mlir/lib/Pass/Pass.cpp
    mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp

Removed: 
    mlir/include/mlir/IR/Identifier.h


################################################################################
diff  --git a/cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp b/cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp
index e3440b5be09ac..677418a537055 100644
--- a/cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp
+++ b/cross-project-tests/debuginfo-tests/llvm-prettyprinters/gdb/mlir-support.cpp
@@ -1,13 +1,12 @@
 #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);

diff  --git a/flang/lib/Lower/FIRBuilder.cpp b/flang/lib/Lower/FIRBuilder.cpp
index 28ced33d9f1bf..b0fcdb6f8d1df 100644
--- a/flang/lib/Lower/FIRBuilder.cpp
+++ b/flang/lib/Lower/FIRBuilder.cpp
@@ -81,7 +81,7 @@ mlir::Value Fortran::lower::FirOpBuilder::allocateLocal(
   });
   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);
 }
@@ -175,9 +175,9 @@ mlir::Value Fortran::lower::FirOpBuilder::createConvert(mlir::Location loc,
 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 =

diff  --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 28a3816c121b6..b9a9316325adc 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -162,7 +162,7 @@ mlir::Value fir::FirOpBuilder::allocateLocal(
   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()) {
@@ -298,9 +298,9 @@ 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},

diff  --git a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
index bb8cd830f1d33..6fb5c745bce21 100644
--- a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
+++ b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp
@@ -238,7 +238,7 @@ TEST_F(FIRBuilderTest, uniqueCFIdent) {
 
 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);
@@ -260,7 +260,7 @@ TEST_F(FIRBuilderTest, hasDynamicSize) {
 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();

diff  --git a/mlir/include/mlir/IR/Attributes.h b/mlir/include/mlir/IR/Attributes.h
index 9fe9f53b1b0c6..d39eb54f775c7 100644
--- a/mlir/include/mlir/IR/Attributes.h
+++ b/mlir/include/mlir/IR/Attributes.h
@@ -15,9 +15,6 @@
 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

diff  --git a/mlir/include/mlir/IR/Builders.h b/mlir/include/mlir/IR/Builders.h
index 31f223972accd..84ec1a55d2383 100644
--- a/mlir/include/mlir/IR/Builders.h
+++ b/mlir/include/mlir/IR/Builders.h
@@ -53,8 +53,6 @@ class Builder {
 
   MLIRContext *getContext() const { return context; }
 
-  StringAttr getIdentifier(const Twine &str);
-
   // Locations.
   Location getUnknownLoc();
   Location getFusedLoc(ArrayRef<Location> locs,

diff  --git a/mlir/include/mlir/IR/BuiltinAttributes.td b/mlir/include/mlir/IR/BuiltinAttributes.td
index aadec07fbbc11..a9b9c2faeca45 100644
--- a/mlir/include/mlir/IR/BuiltinAttributes.td
+++ b/mlir/include/mlir/IR/BuiltinAttributes.td
@@ -949,12 +949,6 @@ def Builtin_StringAttr : Builtin_Attr<"String"> {
       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

diff  --git a/mlir/include/mlir/IR/Identifier.h b/mlir/include/mlir/IR/Identifier.h
deleted file mode 100644
index 0e2574768b14e..0000000000000
--- a/mlir/include/mlir/IR/Identifier.h
+++ /dev/null
@@ -1,20 +0,0 @@
-//===- 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

diff  --git a/mlir/lib/CAPI/IR/IR.cpp b/mlir/lib/CAPI/IR/IR.cpp
index 955f5e0c1eb6c..13b7673e2bccd 100644
--- a/mlir/lib/CAPI/IR/IR.cpp
+++ b/mlir/lib/CAPI/IR/IR.cpp
@@ -820,8 +820,8 @@ MlirLogicalResult mlirSymbolTableReplaceAllSymbolUses(MlirStringRef oldSymbol,
                                                       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)));
 }

diff  --git a/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp b/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
index 14593800b16f1..f78e179911882 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
@@ -1182,12 +1182,12 @@ static void populateVectorizationPatterns(
       ConvOp::getOperationName(), 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;

diff  --git a/mlir/lib/IR/Builders.cpp b/mlir/lib/IR/Builders.cpp
index 3d3e835f4abff..de7204f40979b 100644
--- a/mlir/lib/IR/Builders.cpp
+++ b/mlir/lib/IR/Builders.cpp
@@ -19,10 +19,6 @@
 
 using namespace mlir;
 
-StringAttr Builder::getIdentifier(const Twine &str) {
-  return getStringAttr(str);
-}
-
 //===----------------------------------------------------------------------===//
 // Locations.
 //===----------------------------------------------------------------------===//

diff  --git a/mlir/lib/Pass/Pass.cpp b/mlir/lib/Pass/Pass.cpp
index fff008330c793..5cc84dcba7e97 100644
--- a/mlir/lib/Pass/Pass.cpp
+++ b/mlir/lib/Pass/Pass.cpp
@@ -109,7 +109,7 @@ struct OpPassManagerImpl {
   /// 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;
   }
 

diff  --git a/mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp b/mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp
index 2543b13eae293..354f6724a4995 100644
--- a/mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp
+++ b/mlir/test/lib/Dialect/Linalg/TestLinalgDistribution.cpp
@@ -59,7 +59,7 @@ void TestLinalgDistribution::runOnFunction() {
       distributeTiledLoopsPatterns, getDistributionOptions(),
       LinalgTransformationFilter(
           ArrayRef<StringAttr>{},
-          {StringAttr::get("distributed", funcOp.getContext())})
+          {StringAttr::get(funcOp.getContext(), "distributed")})
           .addFilter([](Operation *op) {
             return success(!op->getParentOfType<linalg::TiledLoopOp>());
           }));


        


More information about the flang-commits mailing list