[IR] "modular-format" attribute for functions using format strings

[mysterymath]

A new InstCombine transform uses this attribute to rewrite calls to a modular version of the implementation along with llvm.reloc.none relocations against aspects of the implementation needed by the call.

This change only adds support for the 'float' aspect, but it also builds the structure needed for others.

See issue #146159

[mysterymath]

Sending this out as a draft to obtain some early feedback about the direction of the implemenation of the Modular Printf RFC.

Prev PR: #147427
Next PR: #147431

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Daniel Thornburgh (mysterymath)

Changes

A new InstCombine transform uses this attribute to rewrite calls to a modular version of the implementation along with llvm.reloc.none relocations against aspects of the implementation needed by the call.

This change only adds support for the 'float' aspect, but it also builds the structure needed for others.

See issue #146159

---

Full diff: https://github.com/llvm/llvm-project/pull/147429.diff

2 Files Affected:

• (modified) llvm/docs/LangRef.rst (+17)
• (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+62)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 187182e5357e7..6694863874241 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -2620,6 +2620,23 @@ For example:
     This attribute indicates that outlining passes should not modify the
     function.
 
+``"modular_format"="<string_idx>,<first_idx_to_check>,<modular_impl_fn>,<impl_name>,<aspects...>"``
+    This attribute indicates that the implementation is modular on a particular
+    format string argument . When the argument for a given call is constant, the
+    compiler may redirect the call to a modular implementation function
+    instead.
+
+    The compiler also emits relocations to report various aspects of the format
+    string and arguments that were present. The compiler reports an aspect by
+    issing a relocation for the symbol `<impl_name>_<aspect>``. This arranges
+    for code and data needed to support the aspect of the implementation to be
+    brought into the link to satisfy weak references in the modular
+    implemenation function.
+
+    The following aspects are currently supported:
+
+    - ``float``: The call has a floating point argument
+
 Call Site Attributes
 ----------------------
 
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index b6ed1dc4331d2..579e5769796c6 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/AssumeBundleQueries.h"
 #include "llvm/Analysis/AssumptionCache.h"
@@ -3915,6 +3916,63 @@ Instruction *InstCombinerImpl::visitCallBrInst(CallBrInst &CBI) {
   return visitCallBase(CBI);
 }
 
+static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) {
+  if (!CI->hasFnAttr("modular-format"))
+    return nullptr;
+
+  SmallVector<StringRef> Args(
+      llvm::split(CI->getFnAttr("modular-format").getValueAsString(), ','));
+  // TODO: Examine the format argument in Args[0].
+  // TODO: Error handling
+  unsigned FirstArgIdx;
+  if (!llvm::to_integer(Args[1], FirstArgIdx))
+    return nullptr;
+  if (FirstArgIdx == 0)
+    return nullptr;
+  --FirstArgIdx;
+  StringRef FnName = Args[2];
+  StringRef ImplName = Args[3];
+  DenseSet<StringRef> Aspects(llvm::from_range,
+                              ArrayRef<StringRef>(Args).drop_front(4));
+  Module *M = CI->getModule();
+  Function *Callee = CI->getCalledFunction();
+  FunctionCallee ModularFn =
+      M->getOrInsertFunction(FnName, Callee->getFunctionType(),
+                             Callee->getAttributes().removeFnAttribute(
+                                 M->getContext(), "modular-format"));
+  CallInst *New = cast<CallInst>(CI->clone());
+  New->setCalledFunction(ModularFn);
+  New->removeFnAttr("modular-format");
+  B.Insert(New);
+
+  const auto ReferenceAspect = [&](StringRef Aspect) {
+    SmallString<20> Name = ImplName;
+    Name += '_';
+    Name += Aspect;
+    Constant *Sym =
+        M->getOrInsertGlobal(Name, Type::getInt8Ty(M->getContext()));
+    Function *RelocNoneFn =
+        Intrinsic::getOrInsertDeclaration(M, Intrinsic::reloc_none);
+    B.CreateCall(RelocNoneFn, {Sym});
+  };
+
+  if (Aspects.contains("float")) {
+    Aspects.erase("float");
+    if (llvm::any_of(
+            llvm::make_range(std::next(CI->arg_begin(), FirstArgIdx),
+                             CI->arg_end()),
+            [](Value *V) { return V->getType()->isFloatingPointTy(); }))
+      ReferenceAspect("float");
+  }
+
+  SmallVector<StringRef> UnknownAspects(Aspects.begin(), Aspects.end());
+  llvm::sort(UnknownAspects);
+  for (StringRef Request : UnknownAspects)
+    ReferenceAspect(Request);
+
+  return New;
+}
+
 Instruction *InstCombinerImpl::tryOptimizeCall(CallInst *CI) {
   if (!CI->getCalledFunction()) return nullptr;
 
@@ -3936,6 +3994,10 @@ Instruction *InstCombinerImpl::tryOptimizeCall(CallInst *CI) {
     ++NumSimplified;
     return CI->use_empty() ? CI : replaceInstUsesWith(*CI, With);
   }
+  if (Value *With = optimizeModularFormat(CI, Builder)) {
+    ++NumSimplified;
+    return CI->use_empty() ? CI : replaceInstUsesWith(*CI, With);
+  }
 
   return nullptr;
 }

[statham-arm]

Is this intended to be specific to printf style format strings? Or will it potentially be reusable for other formatting languages? The text here just says "format string" without saying what kind. For futureproofing, perhaps there should be some kind of a type parameter here saying which formatting mechanism is referred to? That way if two incompatible formatting syntaxes need to be supported later, there's space for expansion.

[mysterymath]

That's a good point and a good idea. There's already a format string type present in the format attribute, so it seems ideal to directly reuse that, especially since the other information is already copied from that attribute, for the same reason. Tying the two would mean that adding support for a new format string type would require adding it in both places, but having a no-op format string type in clang's format implementation doesn't seem like it would be that big of a deal.

It was also momentally tempting to combine this format string type with the impl_name field, but on further thought, this would require that an executable have only one implementation for each format string type. That may commonly be the case, but I can't think of an intrinsic reason it would be so, and keeping them separate would only cost a few extra bytes per annotated call, so it doesn't seem worth it to combine them.

I'll make the change throughout the PR stack and report back.

[statham-arm]

Thanks.

An afterthought: even printf and scanf are different enough that they'd need separate handling, so I don't even need to postulate a hypothetical format string syntax from some other language's standard library. For example, your check for arguments of actual floating-point type is fine for telling that a printf call doesn't need float formatting, but for a scanf call, you'd need to look for pointers to floating types instead.