[Core] Add Twine support for StringAttr and Identifier. NFC.

[lattner]

Summary

This is both more efficient and more ergonomic than going
through an std::string, e.g. when using llvm::utostr and
in string concat cases.

[lattner]

xref: https://github.com/llvm/circt/pull/1202#pullrequestreview-676800939

[jpienaar]

What about just having the twine format given StringRef converts to twine implicitly and there is the getSingleStringRef method that should avoid extra overhead?

[lattner]

I'm open to that, only two downsides I can think of:

1. absolutely tiny additional runtime overhead, slight codebloat for making the temporary twine. I'm not concerned about this.

2. additional header depedency for everything that uses these methods, they'd have to include twine.h. This may already be happening implicitly though.

[rriddle]

In D103754#2801055, @lattner wrote:

I'm open to that, only two downsides I can think of:

1. absolutely tiny additional runtime overhead, slight codebloat for making the temporary twine. I'm not concerned about this.

2. additional header depedency for everything that uses these methods, they'd have to include twine.h. This may already be happening implicitly though.

BuiltinAttributes.h already includes Twine.h transitively, Identifier.h doesn't. Though I would suspect that many uses of Identifier.h also bring in Twine.h through some other means.

[lattner]

Simplify the API for Identifier::get and StringAttr::get to always take Twines.

River pointed out that Twine is effectively pervasive already, so we might as well
just use it.

[lattner]

tweak commit message.

[rriddle]

mlir/lib/IR/MLIRContext.cpp

758

nit: Spell out auto here.

[lattner]

spell out auto

[mehdi_amini]

mlir/lib/IR/MLIRContext.cpp

758

I'm a bit puzzled by this change: this is pessimizing all the clients code that have a string already formed and were able to just pass a StringRef.

I feel that Twine is justified when an API may not have to materialize the string, otherwise in most cases you better hoist everything in the caller and keep the StringRef case fast (especially when it is the common case)

[lattner]

mlir/lib/IR/MLIRContext.cpp

758

What is your concern here? If you pass in something stringref compatible "toStringRef" will do the right thing, it doesn't add a copy of the data into tempStr unless needed.

[mehdi_amini]

mlir/lib/IR/MLIRContext.cpp

758

Ohhhh I didn't know about this toStringRef behavior! I thought the copy always happened...

[lattner]

mlir/lib/IR/MLIRContext.cpp

758

Nope! The vector is only there in case it is necessary, it isn't filled in if not.

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; }
 
-  Identifier getIdentifier(StringRef str);
+  Identifier getIdentifier(const Twine &str);
 
   // Locations.
   Location getUnknownLoc();
   Location getFusedLoc(ArrayRef<Location> locs,
                        Attribute metadata = Attribute());
 
   // Types.
   FloatType getBF16Type();
public:
 
   UnitAttr getUnitAttr();
   BoolAttr getBoolAttr(bool value);
   DictionaryAttr getDictionaryAttr(ArrayRef<NamedAttribute> value);
   IntegerAttr getIntegerAttr(Type type, int64_t value);
   IntegerAttr getIntegerAttr(Type type, const APInt &value);
   FloatAttr getFloatAttr(Type type, double value);
   FloatAttr getFloatAttr(Type type, const APFloat &value);
-  StringAttr getStringAttr(StringRef bytes);
+  StringAttr getStringAttr(const Twine &bytes);
   ArrayAttr getArrayAttr(ArrayRef<Attribute> value);
   FlatSymbolRefAttr getSymbolRefAttr(Operation *value);
   FlatSymbolRefAttr getSymbolRefAttr(StringRef value);
   SymbolRefAttr getSymbolRefAttr(StringRef value,
                                  ArrayRef<FlatSymbolRefAttr> nestedReferences);
 
   // Returns a 0-valued attribute of the given `type`. This function only
   // supports boolean, integer, and 16-/32-/64-bit float types, and vector or
public:
   /// Insert the given operation at the current insertion point and return it.
   Operation *insert(Operation *op);
 
   /// Creates an operation given the fields represented as an OperationState.
   Operation *createOperation(const OperationState &state);
 
   /// Create an operation of specific op type at the current insertion point.
   template <typename OpTy, typename... Args>
-  OpTy create(Location location, Args &&... args) {
+  OpTy create(Location location, Args &&...args) {
     OperationState state(location, OpTy::getOperationName());
     if (!state.name.getAbstractOperation())
       llvm::report_fatal_error("Building op `" +
                                state.name.getStringRef().str() +
                                "` but it isn't registered in this MLIRContext");
     OpTy::build(*this, state, std::forward<Args>(args)...);
     auto *op = createOperation(state);
     auto result = dyn_cast<OpTy>(op);
     assert(result && "builder didn't return the right type");
     return result;
   }
 
   /// Create an operation of specific op type at the current insertion point,
   /// and immediately try to fold it. This functions populates 'results' with
   /// the results after folding the operation.
   template <typename OpTy, typename... Args>
   void createOrFold(SmallVectorImpl<Value> &results, Location location,
-                    Args &&... args) {
+                    Args &&...args) {
     // Create the operation without using 'createOperation' as we don't want to
     // insert it yet.
     OperationState state(location, OpTy::getOperationName());
     if (!state.name.getAbstractOperation())
       llvm::report_fatal_error("Building op `" +
                                state.name.getStringRef().str() +
                                "` but it isn't registered in this MLIRContext");
     OpTy::build(*this, state, std::forward<Args>(args)...);
     Operation *op = Operation::create(state);
 
     // Fold the operation. If successful destroy it, otherwise insert it.
     if (succeeded(tryFold(op, results)))
       op->destroy();
     else
       insert(op);
   }
 
   /// Overload to create or fold a single result operation.
   template <typename OpTy, typename... Args>
   typename std::enable_if<OpTy::template hasTrait<OpTrait::OneResult>(),
                           Value>::type
-  createOrFold(Location location, Args &&... args) {
+  createOrFold(Location location, Args &&...args) {
     SmallVector<Value, 1> results;
     createOrFold<OpTy>(results, location, std::forward<Args>(args)...);
     return results.front();
   }
 
   /// Overload to create or fold a zero result operation.
   template <typename OpTy, typename... Args>
   typename std::enable_if<OpTy::template hasTrait<OpTrait::ZeroResult>(),
                           OpTy>::type
-  createOrFold(Location location, Args &&... args) {
+  createOrFold(Location location, Args &&...args) {
     auto op = create<OpTy>(location, std::forward<Args>(args)...);
     SmallVector<Value, 0> unused;
     tryFold(op.getOperation(), unused);
 
     // Folding cannot remove a zero-result operation, so for convenience we
     // continue to return it.
     return op;
   }

mlir/include/mlir/IR/BuiltinAttributes.td

let description = [{
     ```mlir
     "An important string"
     "string with a type" : !dialect.string
     ```
   }];
   let parameters = (ins StringRefParameter<"">:$value,
                         AttributeSelfTypeParameter<"">:$type);
   let builders = [
-    AttrBuilderWithInferredContext<(ins "StringRef":$bytes,
-                                        "Type":$type), [{
-      return $_get(type.getContext(), bytes, type);
-    }]>,
-    AttrBuilder<(ins "StringRef":$bytes), [{
-      if (bytes.empty())
-        return get($_ctxt);
-      return $_get($_ctxt, bytes, NoneType::get($_ctxt));
-    }]>,
-
+    AttrBuilderWithInferredContext<(ins "const Twine &":$bytes, "Type":$type)>,
+    /// Build an string attr with NoneType.
+    AttrBuilder<(ins "const Twine &":$bytes)>,
     /// Build an empty string attr with NoneType.
     AttrBuilder<(ins)>
   ];
   let extraClassDeclaration = [{
     using ValueType = StringRef;
 
   private:
     /// Return an empty StringAttr with NoneType type. This is a special variant

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/Support/LLVM.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/StringMapEntry.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 
 namespace mlir {
 class Dialect;
 class MLIRContext;
 
 /// This class represents a uniqued string owned by an MLIRContext.  Strings
 /// represented by this type cannot contain nul characters, and may not have a

 /// generic attribute defined by the dialect (in general applicable to multiple
 /// operations).
 class Identifier {
   using EntryType =
       llvm::StringMapEntry<PointerUnion<Dialect *, MLIRContext *>>;
 
 public:
   /// Return an identifier for the specified string.
-  static Identifier get(StringRef str, MLIRContext *context);
+  static Identifier get(const Twine &string, MLIRContext *context);
+
   Identifier(const Identifier &) = default;
   Identifier &operator=(const Identifier &other) = default;
 
   /// Return a StringRef for the string.
   StringRef strref() const { return entry->first(); }
 
   /// Identifiers implicitly convert to StringRefs.
   operator StringRef() const { return strref(); }

mlir/lib/CAPI/IR/BuiltinAttributes.cpp

bool mlirAttributeIsADictionary(MlirAttribute attr) {
   return unwrap(attr).isa<DictionaryAttr>();
 }
 
 MlirAttribute mlirDictionaryAttrGet(MlirContext ctx, intptr_t numElements,
                                     MlirNamedAttribute const *elements) {
   SmallVector<NamedAttribute, 8> attributes;
   attributes.reserve(numElements);
   for (intptr_t i = 0; i < numElements; ++i)
-    attributes.emplace_back(
-        Identifier::get(unwrap(elements[i].name), unwrap(ctx)),
+    attributes.emplace_back(unwrap(elements[i].name),
                             unwrap(elements[i].attribute));
   return wrap(DictionaryAttr::get(unwrap(ctx), attributes));
 }
 
 intptr_t mlirDictionaryAttrGetNumElements(MlirAttribute attr) {
   return static_cast<intptr_t>(unwrap(attr).cast<DictionaryAttr>().size());
 }
 
 MlirNamedAttribute mlirDictionaryAttrGetElement(MlirAttribute attr,

mlir/lib/Conversion/GPUToVulkan/ConvertGPULaunchFuncToVulkanLaunchFunc.cpp

void ConvertGpuLaunchFuncToVulkanLaunchFunc::convertGpuLaunchFunc(
   // Create vulkan launch call op.
   auto vulkanLaunchCallOp = builder.create<CallOp>(
       loc, TypeRange{}, builder.getSymbolRefAttr(kVulkanLaunch),
       vulkanLaunchOperands);
 
   // Set SPIR-V binary shader data as an attribute.
   vulkanLaunchCallOp->setAttr(
       kSPIRVBlobAttrName,
-      StringAttr::get(loc->getContext(), {binary.data(), binary.size()}));
+      StringAttr::get(loc->getContext(),
+                      StringRef(binary.data(), binary.size())));
 
   // Set entry point name as an attribute.
   vulkanLaunchCallOp->setAttr(
       kSPIRVEntryPointAttrName,
       StringAttr::get(loc->getContext(), launchOp.getKernelName()));
 
   launchOp.erase();
 }
 
 std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>>
 mlir::createConvertGpuLaunchFuncToVulkanLaunchFuncPass() {
   return std::make_unique<ConvertGpuLaunchFuncToVulkanLaunchFunc>();
 }

mlir/lib/Dialect/GPU/Transforms/SerializeToBlob.cpp

void gpu::SerializeToBlobPass::runOnOperation() {
   std::string targetISA = translateToISA(*llvmModule, *targetMachine);
 
   // Serialize the target ISA.
   std::unique_ptr<std::vector<char>> blob = serializeISA(targetISA);
   if (!blob)
     return signalPassFailure();
 
   // Add the blob as module attribute.
-  auto attr = StringAttr::get(&getContext(), {blob->data(), blob->size()});
+  auto attr =
+      StringAttr::get(&getContext(), StringRef(blob->data(), blob->size()));
   getOperation()->setAttr(gpuBinaryAnnotation, attr);
 }
 
 void gpu::SerializeToBlobPass::getDependentDialects(
     DialectRegistry &registry) const {
   registerLLVMDialectTranslation(registry);
   OperationPass<gpu::GPUModuleOp>::getDependentDialects(registry);
 }

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

LogicalResult mlir::linalg::LinalgTransformationFilter::checkAndNotify(
   });
 }
 
 void mlir::linalg::LinalgTransformationFilter::
     replaceLinalgTransformationFilter(PatternRewriter &rewriter,
                                       Operation *op) const {
   if (replacement.hasValue())
     op->setAttr(LinalgTransforms::kLinalgTransformMarker,
-                rewriter.getStringAttr(replacement.getValue()));
+                rewriter.getStringAttr(replacement.getValue().strref()));
   else
     op->removeAttr(Identifier::get(LinalgTransforms::kLinalgTransformMarker,
                                    rewriter.getContext()));
 }
 
 LinalgTilingOptions &
 mlir::linalg::LinalgTilingOptions::setTileSizes(ArrayRef<int64_t> ts) {
   SmallVector<int64_t, 4> tileSizes(ts.begin(), ts.end());

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

if (keyword == "if") {
       // Fail if there was already another default clause.
       if (defaultVal)
         return allowedOnce(parser, "default", opName);
       defaultVal = true;
       StringRef defval;
       if (parser.parseLParen() || parser.parseKeyword(&defval) ||
           parser.parseRParen())
         return failure();
-      SmallString<16> attrval;
       // The def prefix is required for the attribute as "private" is a keyword
       // in C++.
-      attrval += "def";
-      attrval += defval;
-      auto attr = parser.getBuilder().getStringAttr(attrval);
+      auto attr = parser.getBuilder().getStringAttr("def" + defval);
       result.addAttribute("default_val", attr);
     } else if (keyword == "proc_bind") {
       // Fail if there was already another proc_bind clause.
       if (procBind)
         return allowedOnce(parser, "proc_bind", opName);
       procBind = true;
       StringRef bind;
       if (parser.parseLParen() || parser.parseKeyword(&bind) ||

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;
 
-Identifier Builder::getIdentifier(StringRef str) {
+Identifier Builder::getIdentifier(const Twine &str) {
   return Identifier::get(str, context);
 }
 
 //===----------------------------------------------------------------------===//
 // Locations.
 //===----------------------------------------------------------------------===//
 
 Location Builder::getUnknownLoc() { return UnknownLoc::get(context); }

 FloatAttr Builder::getFloatAttr(Type type, double value) {
   return FloatAttr::get(type, value);
 }
 
 FloatAttr Builder::getFloatAttr(Type type, const APFloat &value) {
   return FloatAttr::get(type, value);
 }
 
-StringAttr Builder::getStringAttr(StringRef bytes) {
+StringAttr Builder::getStringAttr(const Twine &bytes) {
   return StringAttr::get(context, bytes);
 }
 
 ArrayAttr Builder::getArrayAttr(ArrayRef<Attribute> value) {
   return ArrayAttr::get(context, value);
 }
 
 FlatSymbolRefAttr Builder::getSymbolRefAttr(Operation *value) {

mlir/lib/IR/BuiltinAttributes.cpp

DictionaryAttr::iterator DictionaryAttr::end() const {
   return getValue().end();
 }
 size_t DictionaryAttr::size() const { return getValue().size(); }
 
 DictionaryAttr DictionaryAttr::getEmptyUnchecked(MLIRContext *context) {
   return Base::get(context, ArrayRef<NamedAttribute>());
 }
 
+//===----------------------------------------------------------------------===//
+// StringAttr
+//===----------------------------------------------------------------------===//
+
 StringAttr StringAttr::getEmptyStringAttrUnchecked(MLIRContext *context) {
   return Base::get(context, "", NoneType::get(context));
 }
 
+/// Twine support for StringAttr.
+StringAttr StringAttr::get(MLIRContext *context, const Twine &twine) {
+  // Fast-path empty twine.
+  if (twine.isTriviallyEmpty())
+    return get(context);
+  SmallVector<char, 32> tempStr;
+  return Base::get(context, twine.toStringRef(tempStr), NoneType::get(context));
+}
+
+/// Twine support for StringAttr.
+StringAttr StringAttr::get(const Twine &twine, Type type) {
+  SmallVector<char, 32> tempStr;
+  return Base::get(type.getContext(), twine.toStringRef(tempStr), type);
+}
+
 //===----------------------------------------------------------------------===//
 // FloatAttr
 //===----------------------------------------------------------------------===//
 
 double FloatAttr::getValueAsDouble() const {
   return getValueAsDouble(getValue());
 }
 double FloatAttr::getValueAsDouble(APFloat value) {

mlir/lib/IR/MLIRContext.cpp

const AbstractType &AbstractType::lookup(TypeID typeID, MLIRContext *context) {
   return *it->second;
 }
 
 //===----------------------------------------------------------------------===//
 // Identifier uniquing
 //===----------------------------------------------------------------------===//
 
 /// Return an identifier for the specified string.
-Identifier Identifier::get(StringRef str, MLIRContext *context) {
+Identifier Identifier::get(const Twine &string, MLIRContext *context) {
+  SmallString<32> tempStr;
+  StringRef str = string.toStringRef(tempStr);
+
   // Check invariants after seeing if we already have something in the
   // identifier table - if we already had it in the table, then it already
   // passed invariant checks.
   assert(!str.empty() && "Cannot create an empty identifier");
   assert(str.find('\0') == StringRef::npos &&
          "Cannot create an identifier with a nul character");
 
   auto getDialectOrContext = [&]() {