Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: users/mysterymath/modular-printf/reloc.none
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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...>"``
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@mysterymath mysterymath Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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
issuing 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
----------------------

Expand Down
62 changes: 62 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
Loading