Reduce inlining that had larger binary size impact

[jpienaar]

Summary

On a build with a large number of registered operations this resulted in
~3.4 MB reduction in binary in release mode.

These were found to reduce the size but this change is RFC (it contains changes
related to debugging and error path but not exclusively).

[mehdi_amini]

~3.4 MB reduction in binary in release mode.

I don’t know how to interpret this without the total binary size, what is the percentage?

[mehdi_amini]

In general I am quite concerned about messing with the optimizer this way, this should be extremely exceptional. This'll be ad-hoc, forces a performance tradeoff specific to a given use case, and couple the "heuristic" to the exact compiler you're using (what does it do on Windows? On Mac?).
Have you tried building your project with -Os? -Oz? PGO? FullLTO?

mlir/include/mlir/Support/TypeID.h

192

Isn't this a potentially hot routine?

[jpienaar]

In D145762#4184158, @mehdi_amini wrote:

~3.4 MB reduction in binary in release mode.

I don’t know how to interpret this without the total binary size, what is the percentage?

The total binary size is relevant for user to decide which part they want to optimize and that's where biggest impact will be by doing more application specific removal. But here I'm more interested in the MLIR parts as this reduction corresponds to ~30% of total MLIR codesize (filtering on any file with mlir in it) in the original binary rather than caring about this specific binary.

In D145762#4184165, @mehdi_amini wrote:

In general I am quite concerned about messing with the optimizer this way, this should be extremely exceptional. This'll be ad-hoc, forces a performance tradeoff specific to a given use case, and couple the "heuristic" to the exact compiler you're using (what does it do on Windows? On Mac?).
Have you tried building your project with -Os? -Oz? PGO? FullLTO?

Performance tradeoff wrt debugging and error case was what I wanted to discuss (these were programmatically identified but I don't think all would make sense). I agree that if pure size is goal it's one thing, but these are intended to be performance optimized binaries in general. The debugging or error case seems not specific, inlining vs function call for these paths trades size for speed in exceptional paths. This is similar to me to where we document expectations around verification in production runs, it's just in code :-). PGO would be able to change the heuristic either way wouldn't it? E.g., having the default of the error/debugging cases be marked as cold.

llvm/include/llvm/Support/TypeName.h

 /// This routine may fail on some platforms or for particularly unusual types.
 /// Do not use it for anything other than logging and debugging aids. It isn't
 /// portable or dependendable in any real sense.
 ///
 /// The returned StringRef will point into a static storage duration string.
 /// However, it may not be null terminated and may be some strangely aligned
 /// inner substring of a larger string.
 template <typename DesiredTypeName>
-inline StringRef getTypeName() {
+LLVM_ATTRIBUTE_NOINLINE StringRef getTypeName() {
 #if defined(__clang__) || defined(__GNUC__)
   StringRef Name = __PRETTY_FUNCTION__;
 
   StringRef Key = "DesiredTypeName = ";
   Name = Name.substr(Name.find(Key));
   assert(!Name.empty() && "Unable to find the template parameter!");
   Name = Name.drop_front(Key.size());
 

mlir/include/mlir/IR/Builders.h

Operation *create(Location loc, StringAttr opName, ValueRange operands,
                     TypeRange types = {},
                     ArrayRef<NamedAttribute> attributes = {},
                     BlockRange successors = {},
                     MutableArrayRef<std::unique_ptr<Region>> regions = {});
 
 private:
   /// Helper for sanity checking preconditions for create* methods below.
   template <typename OpT>
-  RegisteredOperationName getCheckRegisteredInfo(MLIRContext *ctx) {
+  LLVM_ATTRIBUTE_NOINLINE RegisteredOperationName
+  getCheckRegisteredInfo(MLIRContext *ctx) {
     std::optional<RegisteredOperationName> opName =
         RegisteredOperationName::lookup(OpT::getOperationName(), ctx);
     if (LLVM_UNLIKELY(!opName)) {
       llvm::report_fatal_error(
           "Building op `" + OpT::getOperationName() +
           "` but it isn't registered in this MLIRContext: the dialect may not "
           "be loaded or this operation isn't registered by the dialect. See "
           "also https://mlir.llvm.org/getting_started/Faq/"

mlir/include/mlir/IR/Diagnostics.h

public:
   MutableArrayRef<DiagnosticArgument> getArguments() { return arguments; }
   ArrayRef<DiagnosticArgument> getArguments() const { return arguments; }
 
   /// Stream operator for inserting new diagnostic arguments.
   template <typename Arg>
   std::enable_if_t<!std::is_convertible<Arg, StringRef>::value &&
                        std::is_constructible<DiagnosticArgument, Arg>::value,
                    Diagnostic &>
-  operator<<(Arg &&val) {
+      LLVM_ATTRIBUTE_NOINLINE operator<<(Arg &&val) {
     arguments.push_back(DiagnosticArgument(std::forward<Arg>(val)));
     return *this;
   }
   Diagnostic &operator<<(StringAttr val);
 
   /// Stream in a string literal.
-  Diagnostic &operator<<(const char *val) {
+  LLVM_ATTRIBUTE_NOINLINE Diagnostic &operator<<(const char *val) {
     arguments.push_back(DiagnosticArgument(val));
     return *this;
   }
 
   /// Stream in a Twine argument.
   Diagnostic &operator<<(char val);
   Diagnostic &operator<<(const Twine &val);
   Diagnostic &operator<<(Twine &&val);
public:
   Diagnostic &appendRange(const T &c, const char *delim = ", ") {
     llvm::interleave(
         c, [this](const auto &a) { *this << a; }, [&]() { *this << delim; });
     return *this;
   }
 
   /// Append arguments to the diagnostic.
   template <typename Arg1, typename Arg2, typename... Args>
-  Diagnostic &append(Arg1 &&arg1, Arg2 &&arg2, Args &&...args) {
+  LLVM_ATTRIBUTE_NOINLINE Diagnostic &append(Arg1 &&arg1, Arg2 &&arg2,
+                                             Args &&...args) {
     append(std::forward<Arg1>(arg1));
     return append(std::forward<Arg2>(arg2), std::forward<Args>(args)...);
   }
   /// Append one argument to the diagnostic.
   template <typename Arg>
   Diagnostic &append(Arg &&arg) {
     *this << std::forward<Arg>(arg);
     return *this;

mlir/include/mlir/IR/OperationSupport.h

struct InterfaceConcept {
     virtual LogicalResult verifyRegionInvariants(Operation *) = 0;
   };
 
 public:
   class Impl : public InterfaceConcept {
   public:
     Impl(StringRef, Dialect *dialect, TypeID typeID,
          detail::InterfaceMap interfaceMap);
+
+    LLVM_ATTRIBUTE_NOINLINE
     Impl(StringAttr name, Dialect *dialect, TypeID typeID,
          detail::InterfaceMap interfaceMap)
         : name(name), typeID(typeID), dialect(dialect),
           interfaceMap(std::move(interfaceMap)) {}
 
+    LLVM_ATTRIBUTE_NOINLINE
+    ~Impl() = default;
+
     /// Returns true if this is a registered operation.
     bool isRegistered() const { return typeID != TypeID::get<void>(); }
     detail::InterfaceMap &getInterfaceMap() { return interfaceMap; }
     Dialect *getDialect() const { return dialect; }
     StringAttr getName() const { return name; }
     TypeID getTypeID() const { return typeID; }
     ArrayRef<StringAttr> getAttributeNames() const { return attributeNames; }
 

 public:
   /// Implementation of the InterfaceConcept for operation APIs that forwarded
   /// to a concrete op implementation.
   template <typename ConcreteOp>
   struct Model : public Impl {
     Model(Dialect *dialect)
         : Impl(ConcreteOp::getOperationName(), dialect,
                TypeID::get<ConcreteOp>(), ConcreteOp::getInterfaceMap()) {}
-    LogicalResult foldHook(Operation *op, ArrayRef<Attribute> attrs,
-                           SmallVectorImpl<OpFoldResult> &results) final {
+    LLVM_ATTRIBUTE_NOINLINE ~Model() = default;
+
+    LLVM_ATTRIBUTE_NOINLINE LogicalResult
+    foldHook(Operation *op, ArrayRef<Attribute> attrs,
       return ConcreteOp::getFoldHookFn()(op, attrs, results);
     }
     void getCanonicalizationPatterns(RewritePatternSet &set,
                                      MLIRContext *context) final {
       ConcreteOp::getCanonicalizationPatterns(set, context);
     }
-    bool hasTrait(TypeID id) final { return ConcreteOp::getHasTraitFn()(id); }
+    LLVM_ATTRIBUTE_NOINLINE bool hasTrait(TypeID id) final {
+      return ConcreteOp::getHasTraitFn()(id);
+    }
     OperationName::ParseAssemblyFn getParseAssemblyFn() final {
       return ConcreteOp::parse;
     }
     void populateDefaultAttrs(const OperationName &name,
                               NamedAttrList &attrs) final {
       ConcreteOp::populateDefaultAttrs(name, attrs);
     }
-    void printAssembly(Operation *op, OpAsmPrinter &printer,
-                       StringRef name) final {
+    LLVM_ATTRIBUTE_NOINLINE void
+    printAssembly(Operation *op, OpAsmPrinter &printer, StringRef name) final {
       ConcreteOp::getPrintAssemblyFn()(op, printer, name);
     }
-    LogicalResult verifyInvariants(Operation *op) final {
+    LLVM_ATTRIBUTE_NOINLINE LogicalResult
+    verifyInvariants(Operation *op) final {
       return ConcreteOp::getVerifyInvariantsFn()(op);
     }
-    LogicalResult verifyRegionInvariants(Operation *op) final {
+    LLVM_ATTRIBUTE_NOINLINE LogicalResult
+    verifyRegionInvariants(Operation *op) final {
       return ConcreteOp::getVerifyRegionInvariantsFn()(op);
     }
   };
 
   /// Lookup the registered operation information for the given operation.
   /// Returns std::nullopt if the operation isn't registered.
   static std::optional<RegisteredOperationName> lookup(StringRef name,
                                                        MLIRContext *ctx);
struct OperationState {
   SmallVector<Type, 4> types;
   NamedAttrList attributes;
   /// Successors of this operation and their respective operands.
   SmallVector<Block *, 1> successors;
   /// Regions that the op will hold.
   SmallVector<std::unique_ptr<Region>, 1> regions;
 
 public:
+  LLVM_ATTRIBUTE_NOINLINE OperationState() = default;
+  LLVM_ATTRIBUTE_NOINLINE ~OperationState() = default;
+
   OperationState(Location location, StringRef name);
   OperationState(Location location, OperationName name);
 
   OperationState(Location location, OperationName name, ValueRange operands,
                  TypeRange types, ArrayRef<NamedAttribute> attributes = {},
                  BlockRange successors = {},
                  MutableArrayRef<std::unique_ptr<Region>> regions = {});
   OperationState(Location location, StringRef name, ValueRange operands,

mlir/include/mlir/IR/PatternMatch.h

insert(LogicalResult (*implFn)(OpType, PatternRewriter &rewriter)) {
     add(std::make_unique<FnPattern>(std::move(implFn), getContext()));
     return *this;
   }
 
 private:
   /// Add an instance of the pattern type 'T'. Return a reference to `this` for
   /// chaining insertions.
   template <typename T, typename... Args>
+  LLVM_ATTRIBUTE_NOINLINE
       std::enable_if_t<std::is_base_of<RewritePattern, T>::value>
       addImpl(ArrayRef<StringRef> debugLabels, Args &&...args) {
     std::unique_ptr<T> pattern =
         RewritePattern::create<T>(std::forward<Args>(args)...);
     pattern->addDebugLabels(debugLabels);
     nativePatterns.emplace_back(std::move(pattern));
   }
   template <typename T, typename... Args>
   std::enable_if_t<std::is_base_of<PDLPatternModule, T>::value>
   addImpl(ArrayRef<StringRef> debugLabels, Args &&...args) {

mlir/include/mlir/Support/TypeID.h

public:
   /// prevent situations where a forward declared type uses this fallback even
   /// though there is a strong definition for the TypeID in the location where
   /// `T` is defined.
   template <typename U>
   using is_fully_resolved_trait = decltype(sizeof(U));
   template <typename U>
   using is_fully_resolved = llvm::is_detected<is_fully_resolved_trait, U>;
 
-  static TypeID resolveTypeID() {
+  LLVM_ATTRIBUTE_NOINLINE static TypeID resolveTypeID() {
     static_assert(is_fully_resolved<T>::value,
                   "TypeID::get<> requires the complete definition of `T`");
     static TypeID id = registerImplicitTypeID(llvm::getTypeName<T>());
     return id;
   }
 };
 
 /// This class provides utilities for resolving the TypeID of a class that