From bb3b826b652c7bed83aa6c06d4af095dcb772592 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Wed, 2 Apr 2025 16:24:57 -0700 Subject: [PATCH 01/10] [IR] "modular-format" attribute for functions using format strings 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 --- llvm/docs/LangRef.rst | 17 +++++ .../InstCombine/InstCombineCalls.cpp | 62 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index fa0b580ee77cb..582f3ae110719 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2742,6 +2742,23 @@ For example: This attribute indicates that outlining passes should not modify the function. +``"modular_format"=",,,,"`` + 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 `_``. 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 e1e24a99d0474..452561570db48 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" @@ -4071,6 +4072,63 @@ Instruction *InstCombinerImpl::visitCallBrInst(CallBrInst &CBI) { return visitCallBase(CBI); } +static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { + if (!CI->hasFnAttr("modular-format")) + return nullptr; + + SmallVector 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 Aspects(llvm::from_range, + ArrayRef(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(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 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; @@ -4092,6 +4150,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; } From 147dad154e235656c3cb3cd1e8552d5ca4d63050 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 8 Jul 2025 15:11:42 -0700 Subject: [PATCH 02/10] issing -> issuing --- llvm/docs/LangRef.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 582f3ae110719..1467e05e295dc 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2750,7 +2750,7 @@ For example: 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 `_``. This arranges + issuing a relocation for the symbol `_``. 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. From c0c98c277c5165b845c09eae4d136533127ac2b4 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Mon, 21 Jul 2025 15:09:58 -0700 Subject: [PATCH 03/10] Emit reloc.none instinsic with metdata string arg --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 452561570db48..b74b326226b79 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4091,11 +4091,12 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { DenseSet Aspects(llvm::from_range, ArrayRef(Args).drop_front(4)); Module *M = CI->getModule(); + LLVMContext &Ctx = M->getContext(); Function *Callee = CI->getCalledFunction(); FunctionCallee ModularFn = M->getOrInsertFunction(FnName, Callee->getFunctionType(), Callee->getAttributes().removeFnAttribute( - M->getContext(), "modular-format")); + Ctx, "modular-format")); CallInst *New = cast(CI->clone()); New->setCalledFunction(ModularFn); New->removeFnAttr("modular-format"); @@ -4105,11 +4106,10 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { 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}); + B.CreateCall(RelocNoneFn, + {MetadataAsValue::get(Ctx, MDString::get(Ctx, Name))}); }; if (Aspects.contains("float")) { From 129b1a4ad4905179d898649fa82bbbbfae1bbcf2 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 22 Jul 2025 13:24:20 -0700 Subject: [PATCH 04/10] Correct modular_format to modular-format in docs --- llvm/docs/LangRef.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 1467e05e295dc..e703f3f5479e5 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2742,7 +2742,7 @@ For example: This attribute indicates that outlining passes should not modify the function. -``"modular_format"=",,,,"`` +``"modular-format"=",,,,"`` 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 From 9dae862346935dd74e91c46b4295854edaa1b8ac Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 22 Jul 2025 13:26:20 -0700 Subject: [PATCH 05/10] Describe the semantics of the arguments copied from C format attr --- llvm/docs/LangRef.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index e703f3f5479e5..95ba5524b6f6e 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2755,6 +2755,9 @@ For example: brought into the link to satisfy weak references in the modular implemenation function. + The first two arguments have the same semantics as the arguments to the C + ``format`` attribute. + The following aspects are currently supported: - ``float``: The call has a floating point argument From 4f489c1ddc032594cd3b34f69e473acb01aec961 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 22 Jul 2025 13:29:09 -0700 Subject: [PATCH 06/10] Add a type arg --- llvm/docs/LangRef.rst | 6 ++++-- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 95ba5524b6f6e..0d70fa8e68faf 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2742,7 +2742,7 @@ For example: This attribute indicates that outlining passes should not modify the function. -``"modular-format"=",,,,"`` +``"modular-format"=",,,,,"`` 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 @@ -2755,13 +2755,15 @@ For example: brought into the link to satisfy weak references in the modular implemenation function. - The first two arguments have the same semantics as the arguments to the C + The first three arguments have the same semantics as the arguments to the C ``format`` attribute. 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 b74b326226b79..22de711041059 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4078,18 +4078,18 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { SmallVector Args( llvm::split(CI->getFnAttr("modular-format").getValueAsString(), ',')); - // TODO: Examine the format argument in Args[0]. + // TODO: Make use of the first two arguments // TODO: Error handling unsigned FirstArgIdx; - if (!llvm::to_integer(Args[1], FirstArgIdx)) + if (!llvm::to_integer(Args[2], FirstArgIdx)) return nullptr; if (FirstArgIdx == 0) return nullptr; --FirstArgIdx; - StringRef FnName = Args[2]; - StringRef ImplName = Args[3]; + StringRef FnName = Args[3]; + StringRef ImplName = Args[4]; DenseSet Aspects(llvm::from_range, - ArrayRef(Args).drop_front(4)); + ArrayRef(Args).drop_front(5)); Module *M = CI->getModule(); LLVMContext &Ctx = M->getContext(); Function *Callee = CI->getCalledFunction(); From 821f9a9c9f5c9e96768699e78ce4b541b4eb50fb Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Thu, 21 Aug 2025 14:49:14 -0700 Subject: [PATCH 07/10] llvm.reloc.none takes a GlobalValue again This avoids avoid modifying Module in ISel --- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 22de711041059..73f2aeb8fecef 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4091,12 +4091,11 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { DenseSet Aspects(llvm::from_range, ArrayRef(Args).drop_front(5)); Module *M = CI->getModule(); - LLVMContext &Ctx = M->getContext(); Function *Callee = CI->getCalledFunction(); FunctionCallee ModularFn = M->getOrInsertFunction(FnName, Callee->getFunctionType(), Callee->getAttributes().removeFnAttribute( - Ctx, "modular-format")); + M->getContext(), "modular-format")); CallInst *New = cast(CI->clone()); New->setCalledFunction(ModularFn); New->removeFnAttr("modular-format"); @@ -4106,10 +4105,11 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { 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, - {MetadataAsValue::get(Ctx, MDString::get(Ctx, Name))}); + B.CreateCall(RelocNoneFn, {Sym}); }; if (Aspects.contains("float")) { From acaf9138641129737f1ad25f91f0bde84fe79d9e Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 26 Aug 2025 14:28:34 -0700 Subject: [PATCH 08/10] Add test cases and polish change --- llvm/docs/LangRef.rst | 21 ++-- llvm/lib/IR/Verifier.cpp | 14 +++ .../InstCombine/InstCombineCalls.cpp | 8 +- .../Transforms/InstCombine/modular-format.ll | 104 ++++++++++++++++++ llvm/test/Verifier/modular-format.ll | 41 +++++++ 5 files changed, 172 insertions(+), 16 deletions(-) create mode 100644 llvm/test/Transforms/InstCombine/modular-format.ll create mode 100644 llvm/test/Verifier/modular-format.ll diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 0d70fa8e68faf..a36852ad18170 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2742,18 +2742,17 @@ For example: This attribute indicates that outlining passes should not modify the function. -``"modular-format"=",,,,,"`` +``"modular-format"=",,,,,"`` 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 `_``. 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. + format string argument. If the compiler can determine that not all aspects + of the implementation are needed, it can report which aspects were needed + and redirect the call to a modular implementation function instead. + + The compiler reports that an implementation aspect is needed by issuing a + relocation for the symbol `_``. 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 first three arguments have the same semantics as the arguments to the C ``format`` attribute. diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 2e08c142c76b9..f26ccc128fe4b 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2554,6 +2554,20 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs, CheckFailed("invalid value for 'denormal-fp-math-f32' attribute: " + S, V); } + + if (auto A = Attrs.getFnAttr("modular-format"); A.isValid()) { + StringRef S = A.getValueAsString(); + SmallVector Args; + S.split(Args, ','); + Check(Args.size() >= 5, + "modular-format attribute requires at least 5 arguments", V); + unsigned FirstArgIdx; + Check(!Args[2].getAsInteger(10, FirstArgIdx), + "modular-format attribute first arg index is not an integer", V); + unsigned UpperBound = FT->getNumParams() + (FT->isVarArg() ? 1 : 0); + Check(FirstArgIdx > 0 && FirstArgIdx <= UpperBound, + "modular-format attribute first arg index is out of bounds", V); + } } void Verifier::verifyUnknownProfileMetadata(MDNode *MD) { Check(MD->getNumOperands() == 2, diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 73f2aeb8fecef..6f71dcdccb2ec 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4079,12 +4079,10 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { SmallVector Args( llvm::split(CI->getFnAttr("modular-format").getValueAsString(), ',')); // TODO: Make use of the first two arguments - // TODO: Error handling unsigned FirstArgIdx; - if (!llvm::to_integer(Args[2], FirstArgIdx)) - return nullptr; - if (FirstArgIdx == 0) - return nullptr; + [[maybe_unused]] bool Error; + Error = Args[2].getAsInteger(10, FirstArgIdx); + assert(!Error && "invalid first arg index"); --FirstArgIdx; StringRef FnName = Args[3]; StringRef ImplName = Args[4]; diff --git a/llvm/test/Transforms/InstCombine/modular-format.ll b/llvm/test/Transforms/InstCombine/modular-format.ll new file mode 100644 index 0000000000000..9b1e60bbab4f8 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/modular-format.ll @@ -0,0 +1,104 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; Test that the modular format string library call simplifier works correctly. +; +; RUN: opt < %s -passes=instcombine -S | FileCheck %s + +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128" + +@.str.int = constant [3 x i8] c"%d\00" +@.str.float = constant [3 x i8] c"%f\00" +@.str.multi = constant [6 x i8] c"%f %d\00" +@.str.multifp = constant [6 x i8] c"%f %f\00" +@.str.noargs = constant [1 x i8] c"\00" + +; Basic Transformation +define void @test_basic(i32 %arg) { +; CHECK-LABEL: @test_basic( +; CHECK-NEXT: call void (ptr, ...) @basic_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @basic(ptr @.str.int, i32 %arg) + ret void +} + +declare void @basic(ptr, ...) "modular-format"="printf,1,2,basic_mod,basic_impl" +; "float" Aspect - Present +define void @test_float_present(double %arg) { +; CHECK-LABEL: @test_float_present( +; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.float, double [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @float_present(ptr @.str.float, double %arg) + ret void +} + +declare void @float_present(ptr, ...) #0 + +; Unknown Aspects +define void @test_unknown_aspects(i32 %arg) { +; CHECK-LABEL: @test_unknown_aspects( +; CHECK-NEXT: call void (ptr, ...) @unknown_aspects_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown1) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown2) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @unknown_aspects(ptr @.str.int, i32 %arg) + ret void +} + +declare void @unknown_aspects(ptr, ...) "modular-format"="printf,1,2,unknown_aspects_mod,basic_impl,unknown1,unknown2" + +; Multiple Aspects +define void @test_multiple_aspects(double %arg1, i32 %arg2) { +; CHECK-LABEL: @test_multiple_aspects( +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.multi, double [[ARG1:%.*]], i32 [[ARG2:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @multiple_aspects(ptr @.str.multi, double %arg1, i32 %arg2) + ret void +} + +declare void @multiple_aspects(ptr, ...) "modular-format"="printf,1,2,multiple_aspects_mod,basic_impl,float,unknown" + +; Multiple Floating-Point Arguments +define void @test_multiple_fp_args(double %arg1, float %arg2) { +; CHECK-LABEL: @test_multiple_fp_args( +; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.multifp, double [[ARG1:%.*]], float [[ARG2:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @multiple_fp_args(ptr @.str.multifp, double %arg1, float %arg2) + ret void +} + +declare void @multiple_fp_args(ptr, ...) #0 + +; No Arguments to Check +define void @test_no_args_to_check() { +; CHECK-LABEL: @test_no_args_to_check( +; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.noargs) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @no_args_to_check(ptr @.str.noargs) + ret void +} + +declare void @no_args_to_check(ptr, ...) #0 + +; First argument index != 2 +define void @test_first_arg_idx(i32 %ignored, double %arg) { +; CHECK-LABEL: @test_first_arg_idx( +; CHECK-NEXT: call void (i32, ptr, ...) @first_arg_idx_mod(i32 [[IGNORED:%.*]], ptr nonnull @.str.float, double [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: ret void +; + call void (i32, ptr, ...) @first_arg_idx(i32 %ignored, ptr @.str.float, double %arg) + ret void +} + +declare void @first_arg_idx(i32, ptr, ...) "modular-format"="printf,2,3,first_arg_idx_mod,basic_impl,float" + +attributes #0 = { "modular-format"="printf,1,2,float_present_mod,basic_impl,float" } diff --git a/llvm/test/Verifier/modular-format.ll b/llvm/test/Verifier/modular-format.ll new file mode 100644 index 0000000000000..abdd73d098be1 --- /dev/null +++ b/llvm/test/Verifier/modular-format.ll @@ -0,0 +1,41 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +define void @test_too_few_arguments(i32 %arg, ...) "modular-format"="printf,1,2,basic_mod" { + ret void +} +; CHECK: modular-format attribute requires at least 5 arguments +; CHECK-NEXT: ptr @test_too_few_arguments + +define void @test_first_arg_index_not_integer(i32 %arg, ...) "modular-format"="printf,1,foo,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute first arg index is not an integer +; CHECK-NEXT: ptr @test_first_arg_index_not_integer + +define void @test_first_arg_index_zero(i32 %arg) "modular-format"="printf,1,0,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute first arg index is out of bounds +; CHECK-NEXT: ptr @test_first_arg_index_zero + +define void @test_first_arg_index_out_of_bounds(i32 %arg) "modular-format"="printf,1,2,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute first arg index is out of bounds +; CHECK-NEXT: ptr @test_first_arg_index_out_of_bounds + +define void @test_first_arg_index_out_of_bounds_varargs(i32 %arg, ...) "modular-format"="printf,1,3,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute first arg index is out of bounds +; CHECK-NEXT: ptr @test_first_arg_index_out_of_bounds_varargs + +; CHECK-NOT: ptr @test_first_arg_index_in_bounds +define void @test_first_arg_index_in_bounds(i32 %arg) "modular-format"="printf,1,1,basic_mod,basic_impl" { + ret void +} + +; CHECK-NOT: ptr @test_first_arg_index_in_bounds_varargs +define void @test_first_arg_index_in_bounds_varargs(i32 %arg, ...) "modular-format"="printf,1,2,basic_mod,basic_impl" { + ret void +} From 7fbc2a9d7e81bf60ceea6d6eda28f6ac49f08215 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Tue, 9 Sep 2025 15:11:04 -0700 Subject: [PATCH 09/10] Don't transform calls unless some aspect is unneeded --- .../InstCombine/InstCombineCalls.cpp | 38 +++++--- .../Transforms/InstCombine/modular-format.ll | 89 ++++++++++--------- 2 files changed, 69 insertions(+), 58 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 6f71dcdccb2ec..913ab31297b06 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4086,8 +4086,28 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { --FirstArgIdx; StringRef FnName = Args[3]; StringRef ImplName = Args[4]; - DenseSet Aspects(llvm::from_range, - ArrayRef(Args).drop_front(5)); + ArrayRef AllAspects = ArrayRef(Args).drop_front(5); + + if (AllAspects.empty()) + return nullptr; + + SmallVector NeededAspects; + for (StringRef Aspect : AllAspects) { + if (Aspect == "float") { + if (llvm::any_of( + llvm::make_range(std::next(CI->arg_begin(), FirstArgIdx), + CI->arg_end()), + [](Value *V) { return V->getType()->isFloatingPointTy(); })) + NeededAspects.push_back("float"); + } else { + // Unknown aspects are always considered to be needed. + NeededAspects.push_back(Aspect); + } + } + + if (NeededAspects.size() == AllAspects.size()) + return nullptr; + Module *M = CI->getModule(); Function *Callee = CI->getCalledFunction(); FunctionCallee ModularFn = @@ -4110,18 +4130,8 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { 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 UnknownAspects(Aspects.begin(), Aspects.end()); - llvm::sort(UnknownAspects); - for (StringRef Request : UnknownAspects) + llvm::sort(NeededAspects); + for (StringRef Request : NeededAspects) ReferenceAspect(Request); return New; diff --git a/llvm/test/Transforms/InstCombine/modular-format.ll b/llvm/test/Transforms/InstCombine/modular-format.ll index 9b1e60bbab4f8..f95b6c5361ae6 100644 --- a/llvm/test/Transforms/InstCombine/modular-format.ll +++ b/llvm/test/Transforms/InstCombine/modular-format.ll @@ -8,75 +8,58 @@ target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f3 @.str.int = constant [3 x i8] c"%d\00" @.str.float = constant [3 x i8] c"%f\00" @.str.multi = constant [6 x i8] c"%f %d\00" -@.str.multifp = constant [6 x i8] c"%f %f\00" @.str.noargs = constant [1 x i8] c"\00" -; Basic Transformation +; No aspects are specified, so no transformation occurs. define void @test_basic(i32 %arg) { ; CHECK-LABEL: @test_basic( -; CHECK-NEXT: call void (ptr, ...) @basic_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) +; CHECK-NEXT: call void (ptr, ...) @basic(ptr nonnull @.str.int, i32 [[ARG:%.*]]) ; CHECK-NEXT: ret void ; call void (ptr, ...) @basic(ptr @.str.int, i32 %arg) ret void } -declare void @basic(ptr, ...) "modular-format"="printf,1,2,basic_mod,basic_impl" -; "float" Aspect - Present +declare void @basic(ptr, ...) #0 + +; The "float" aspect is present and needed, so no transformation occurs. define void @test_float_present(double %arg) { ; CHECK-LABEL: @test_float_present( -; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.float, double [[ARG:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: call void (ptr, ...) @float_present(ptr nonnull @.str.float, double [[ARG:%.*]]) ; CHECK-NEXT: ret void ; call void (ptr, ...) @float_present(ptr @.str.float, double %arg) ret void } -declare void @float_present(ptr, ...) #0 +declare void @float_present(ptr, ...) #1 -; Unknown Aspects -define void @test_unknown_aspects(i32 %arg) { -; CHECK-LABEL: @test_unknown_aspects( -; CHECK-NEXT: call void (ptr, ...) @unknown_aspects_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown1) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown2) -; CHECK-NEXT: ret void -; - call void (ptr, ...) @unknown_aspects(ptr @.str.int, i32 %arg) - ret void -} - -declare void @unknown_aspects(ptr, ...) "modular-format"="printf,1,2,unknown_aspects_mod,basic_impl,unknown1,unknown2" - -; Multiple Aspects -define void @test_multiple_aspects(double %arg1, i32 %arg2) { -; CHECK-LABEL: @test_multiple_aspects( -; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.multi, double [[ARG1:%.*]], i32 [[ARG2:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown) +; The "float" aspect is present but not needed, so the call is transformed. +define void @test_float_absent(i32 %arg) { +; CHECK-LABEL: @test_float_absent( +; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) ; CHECK-NEXT: ret void ; - call void (ptr, ...) @multiple_aspects(ptr @.str.multi, double %arg1, i32 %arg2) + call void (ptr, ...) @float_absent(ptr @.str.int, i32 %arg) ret void } -declare void @multiple_aspects(ptr, ...) "modular-format"="printf,1,2,multiple_aspects_mod,basic_impl,float,unknown" +declare void @float_absent(ptr, ...) #1 -; Multiple Floating-Point Arguments -define void @test_multiple_fp_args(double %arg1, float %arg2) { -; CHECK-LABEL: @test_multiple_fp_args( -; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.multifp, double [[ARG1:%.*]], float [[ARG2:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; Unknown aspects are always considered needed, so no transformation occurs. +define void @test_unknown_aspects(i32 %arg) { +; CHECK-LABEL: @test_unknown_aspects( +; CHECK-NEXT: call void (ptr, ...) @unknown_aspects(ptr nonnull @.str.int, i32 [[ARG:%.*]]) ; CHECK-NEXT: ret void ; - call void (ptr, ...) @multiple_fp_args(ptr @.str.multifp, double %arg1, float %arg2) + call void (ptr, ...) @unknown_aspects(ptr @.str.int, i32 %arg) ret void } -declare void @multiple_fp_args(ptr, ...) #0 +declare void @unknown_aspects(ptr, ...) #2 -; No Arguments to Check +; The call has no arguments to check, so the "float" aspect is not needed and +; the call is transformed. define void @test_no_args_to_check() { ; CHECK-LABEL: @test_no_args_to_check( ; CHECK-NEXT: call void (ptr, ...) @float_present_mod(ptr nonnull @.str.noargs) @@ -86,19 +69,37 @@ define void @test_no_args_to_check() { ret void } -declare void @no_args_to_check(ptr, ...) #0 +declare void @no_args_to_check(ptr, ...) #1 -; First argument index != 2 +; The first argument index is not 2. The "float" aspect is needed, so no +; transformation occurs. define void @test_first_arg_idx(i32 %ignored, double %arg) { ; CHECK-LABEL: @test_first_arg_idx( -; CHECK-NEXT: call void (i32, ptr, ...) @first_arg_idx_mod(i32 [[IGNORED:%.*]], ptr nonnull @.str.float, double [[ARG:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_float) +; CHECK-NEXT: call void (i32, ptr, ...) @first_arg_idx(i32 [[IGNORED:%.*]], ptr nonnull @.str.float, double [[ARG:%.*]]) ; CHECK-NEXT: ret void ; call void (i32, ptr, ...) @first_arg_idx(i32 %ignored, ptr @.str.float, double %arg) ret void } -declare void @first_arg_idx(i32, ptr, ...) "modular-format"="printf,2,3,first_arg_idx_mod,basic_impl,float" +declare void @first_arg_idx(i32, ptr, ...) #3 + +; One aspect ("unknown") is needed, but one ("float") is not. The call is +; transformed, and a reference to the needed aspect is emitted. +define void @test_partial_aspects(i32 %arg) { +; CHECK-LABEL: @test_partial_aspects( +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown) +; CHECK-NEXT: ret void +; + call void (ptr, ...) @partial_aspects(ptr @.str.int, i32 %arg) + ret void +} + +declare void @partial_aspects(ptr, ...) #4 -attributes #0 = { "modular-format"="printf,1,2,float_present_mod,basic_impl,float" } +attributes #0 = { "modular-format"="printf,1,2,basic_mod,basic_impl" } +attributes #1 = { "modular-format"="printf,1,2,float_present_mod,basic_impl,float" } +attributes #2 = { "modular-format"="printf,1,2,unknown_aspects_mod,basic_impl,unknown1,unknown2" } +attributes #3 = { "modular-format"="printf,2,3,first_arg_idx_mod,basic_impl,float" } +attributes #4 = { "modular-format"="printf,1,2,multiple_aspects_mod,basic_impl,float,unknown" } From c0038c2566f1ce22772f703eda6852688d0f6dac Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh Date: Thu, 16 Oct 2025 15:36:05 -0700 Subject: [PATCH 10/10] Revert "llvm.reloc.none takes a GlobalValue again" This reverts commit 4ac826513d3f564341d91bb9bc5cbf3a53770b57. --- .../lib/Transforms/InstCombine/InstCombineCalls.cpp | 13 ++++++------- llvm/test/Transforms/InstCombine/modular-format.ll | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 913ab31297b06..e10edc229b0a2 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -4109,11 +4109,11 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { return nullptr; Module *M = CI->getModule(); + LLVMContext &Ctx = M->getContext(); Function *Callee = CI->getCalledFunction(); - FunctionCallee ModularFn = - M->getOrInsertFunction(FnName, Callee->getFunctionType(), - Callee->getAttributes().removeFnAttribute( - M->getContext(), "modular-format")); + FunctionCallee ModularFn = M->getOrInsertFunction( + FnName, Callee->getFunctionType(), + Callee->getAttributes().removeFnAttribute(Ctx, "modular-format")); CallInst *New = cast(CI->clone()); New->setCalledFunction(ModularFn); New->removeFnAttr("modular-format"); @@ -4123,11 +4123,10 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { 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}); + B.CreateCall(RelocNoneFn, + {MetadataAsValue::get(Ctx, MDString::get(Ctx, Name))}); }; llvm::sort(NeededAspects); diff --git a/llvm/test/Transforms/InstCombine/modular-format.ll b/llvm/test/Transforms/InstCombine/modular-format.ll index f95b6c5361ae6..af45942cc33df 100644 --- a/llvm/test/Transforms/InstCombine/modular-format.ll +++ b/llvm/test/Transforms/InstCombine/modular-format.ll @@ -89,7 +89,7 @@ declare void @first_arg_idx(i32, ptr, ...) #3 define void @test_partial_aspects(i32 %arg) { ; CHECK-LABEL: @test_partial_aspects( ; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.int, i32 [[ARG:%.*]]) -; CHECK-NEXT: call void @llvm.reloc.none(ptr nonnull @basic_impl_unknown) +; CHECK-NEXT: call void @llvm.reloc.none(metadata !"basic_impl_unknown") ; CHECK-NEXT: ret void ; call void (ptr, ...) @partial_aspects(ptr @.str.int, i32 %arg)