https://github.com/mysterymath updated https://github.com/llvm/llvm-project/pull/202425
>From 72f1526cb04e11b7e429d17f2029b67eaa15cc43 Mon Sep 17 00:00:00 2001 From: Daniel Thornburgh <[email protected]> Date: Thu, 21 May 2026 13:22:44 -0700 Subject: [PATCH] [LLVM][InstCombine] Add fixed-point aspect to modular printf parser This commit adds support for the "fixed" aspect inside LLVM's modular printf optimizer pass (InstCombineCalls.cpp). It implements a targeted format string parser that scans constant format strings for "%r, %R, %k, %K" specifiers to resolve if the "fixed" aspect is needed, falling back to conservatively assumed needed if the format string is dynamic. This commit also adds missing Verifier checks for modular-format, including those that guard the parts of its syntax that are newly used by this change. --- clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/Basic/AttrDocs.td | 3 +- llvm/docs/LangRef.rst | 3 +- llvm/docs/ReleaseNotes.md | 3 + llvm/lib/IR/Verifier.cpp | 13 +- .../InstCombine/InstCombineCalls.cpp | 122 ++++++++++++++---- .../Transforms/InstCombine/modular-format.ll | 74 +++++++++++ llvm/test/Verifier/modular-format.ll | 30 +++++ 8 files changed, 220 insertions(+), 31 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index cf4826f50e5a5..649eeaece1fa6 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -462,6 +462,9 @@ Attribute Changes in Clang about pointer lifetimes. It may be used to power optimizations in the future, however there are no concrete plans to do so at the moment. +* The ``modular_format`` attribute now supports the ``fixed`` aspect for C + N1169 fixed-point ``printf`` specifiers. + Improvements to Clang's diagnostics ----------------------------------- - Fixed bug in ``-Wdocumentation`` so that it correctly handles explicit diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index f54f8e8f90596..dc3bea36738ea 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -10013,7 +10013,8 @@ excepting that the aspects may be in any order. The following aspects are currently supported: -- ``float``: The call has a floating point argument +- ``fixed``: The call has a C N1169 fixed-point argument. +- ``float``: The call has a floating-point argument. }]; } diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 5e5ebdf80773f..c0b3e1c1d09aa 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -2852,7 +2852,8 @@ For example: The following aspects are currently supported: - - ``float``: The call has a floating point argument + - ``fixed``: The call has a C N1169 fixed-point argument. + - ``float``: The call has a floating-point argument. diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 5cd88f4260011..f4e07aac53e6a 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -89,6 +89,9 @@ Makes programs 10x faster by doing Special New Thing. * Fast math flags are now permitted on `uitofp` and `sitofp`. +* The ``modular-format`` attribute now supports the ``fixed`` aspect for C + N1169 fixed-point ``printf`` specifiers. + ### Changes to LLVM infrastructure * Removed ``Constant::isZeroValue``. It was functionally identical to diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index c9639d1420bfc..9c208c45bd4ac 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -2729,12 +2729,23 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs, S.split(Args, ','); Check(Args.size() >= 5, "modular-format attribute requires at least 5 arguments", V); + unsigned UpperBound = FT->getNumParams() + (FT->isVarArg() ? 1 : 0); + unsigned FormatIdx; + Check(!Args[1].getAsInteger(10, FormatIdx), + "modular-format attribute format string index is not an integer", V); + Check(FormatIdx > 0, + "modular-format attribute format string index must be greater than 0", V); + Check(FormatIdx <= UpperBound, + "modular-format attribute format string index is out of bounds", 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 <= UpperBound, "modular-format attribute first arg index is out of bounds", V); + Check(!Args[3].empty(), + "modular-format attribute modular implementation function name cannot be empty", V); + Check(!Args[4].empty(), + "modular-format attribute implementation name cannot be empty", V); } if (auto A = Attrs.getFnAttr("target-features"); A.isValid()) { diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index a4db002b1eec2..57714df00e10a 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -15,11 +15,13 @@ #include "llvm/ADT/APInt.h" #include "llvm/ADT/APSInt.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Bitset.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/Analysis/AliasAnalysis.h" #include "llvm/Analysis/AssumeBundleQueries.h" #include "llvm/Analysis/AssumptionCache.h" @@ -4380,40 +4382,114 @@ Instruction *InstCombinerImpl::visitCallBrInst(CallBrInst &CBI) { return visitCallBase(CBI); } +// A simple parser for format string specifiers for the purposes of the +// modular-format attribute. In the case of malformed format strings this might +// under or over report the specifiers present, but such cases are undefined +// behavior. +static Bitset<256> parseFormatStringSpecifiers(StringRef FormatStr) { + Bitset<256> Specifiers; + for (size_t I = 0; I < FormatStr.size(); ++I) { + if (FormatStr[I] != '%') + continue; + + // Check for escaped '%'. + if (I + 1 < FormatStr.size() && FormatStr[I + 1] == '%') { + ++I; // Skip the second '%'. + continue; + } + + // Scan past allowed prefix characters. + size_t J = + FormatStr.find_first_not_of("0123456789-+ #0$.*'hlLjztqwvI", I + 1); + if (J == StringRef::npos) + break; + + Specifiers.set(static_cast<unsigned char>(FormatStr[J])); + I = J; // Resume search from after the specifier. + } + return Specifiers; +} + +static bool isAspectNeeded(StringRef Aspect, CallInst *CI, unsigned FirstArgIdx, + const std::optional<Bitset<256>> &Specifiers) { + if (Aspect == "float") { + if (Specifiers) { + static constexpr Bitset<256> FloatSpecifiers{'f', 'F', 'e', 'E', + 'g', 'G', 'a', 'A'}; + return (*Specifiers & FloatSpecifiers).any(); + } + // Fallback to type-based check for dynamic format string. + return llvm::any_of( + llvm::make_range(std::next(CI->arg_begin(), FirstArgIdx), + CI->arg_end()), + [](Value *V) { return V->getType()->isFloatingPointTy(); }); + } + if (Aspect == "fixed") { + if (Specifiers) { + static constexpr Bitset<256> FixedSpecifiers{'r', 'R', 'k', 'K'}; + return (*Specifiers & FixedSpecifiers).any(); + } + // Fallback for fixed-point: assume needed if format is dynamic. + return true; + } + // Unknown aspects are always considered to be needed. + return true; +} + +static void referenceAspect(StringRef Aspect, StringRef ImplName, Module *M, + IRBuilderBase &B) { + SmallString<20> Name = ImplName; + Name += '_'; + Name += Aspect; + LLVMContext &Ctx = M->getContext(); + Function *RelocNoneFn = + Intrinsic::getOrInsertDeclaration(M, Intrinsic::reloc_none); + B.CreateCall(RelocNoneFn, + {MetadataAsValue::get(Ctx, MDString::get(Ctx, Name))}); +} + 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: Make use of the first two arguments + if (Args.size() < 5) + return nullptr; + + StringRef FormatIdxStr = Args[1]; + StringRef FirstArgIdxStr = Args[2]; + StringRef FnName = Args[3]; + StringRef ImplName = Args[4]; + ArrayRef<StringRef> AllAspects = ArrayRef<StringRef>(Args).drop_front(5); + + unsigned FormatIdx; unsigned FirstArgIdx; [[maybe_unused]] bool Error; - Error = Args[2].getAsInteger(10, FirstArgIdx); + Error = FormatIdxStr.getAsInteger(10, FormatIdx); + assert(!Error && "invalid format arg index"); + --FormatIdx; // 1-based to 0-based + + Error = FirstArgIdxStr.getAsInteger(10, FirstArgIdx); assert(!Error && "invalid first arg index"); if (FirstArgIdx == 0) return nullptr; - --FirstArgIdx; - StringRef FnName = Args[3]; - StringRef ImplName = Args[4]; - ArrayRef<StringRef> AllAspects = ArrayRef<StringRef>(Args).drop_front(5); + --FirstArgIdx; // 1-based to 0-based if (AllAspects.empty()) return nullptr; + Value *FormatVal = CI->getArgOperand(FormatIdx); + StringRef FormatStr; + + std::optional<Bitset<256>> Specifiers; + if (getConstantStringInfo(FormatVal, FormatStr)) + Specifiers = parseFormatStringSpecifiers(FormatStr); + SmallVector<StringRef> 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. + for (StringRef Aspect : AllAspects) + if (isAspectNeeded(Aspect, CI, FirstArgIdx, Specifiers)) NeededAspects.push_back(Aspect); - } - } if (NeededAspects.size() == AllAspects.size()) return nullptr; @@ -4429,19 +4505,9 @@ static Value *optimizeModularFormat(CallInst *CI, IRBuilderBase &B) { New->removeFnAttr("modular-format"); B.Insert(New); - const auto ReferenceAspect = [&](StringRef Aspect) { - SmallString<20> Name = ImplName; - Name += '_'; - Name += Aspect; - Function *RelocNoneFn = - Intrinsic::getOrInsertDeclaration(M, Intrinsic::reloc_none); - B.CreateCall(RelocNoneFn, - {MetadataAsValue::get(Ctx, MDString::get(Ctx, Name))}); - }; - llvm::sort(NeededAspects); for (StringRef Request : NeededAspects) - ReferenceAspect(Request); + referenceAspect(Request, ImplName, M, B); return New; } diff --git a/llvm/test/Transforms/InstCombine/modular-format.ll b/llvm/test/Transforms/InstCombine/modular-format.ll index 79c0c2a263675..f7d877013094b 100644 --- a/llvm/test/Transforms/InstCombine/modular-format.ll +++ b/llvm/test/Transforms/InstCombine/modular-format.ll @@ -111,9 +111,83 @@ define void @test_zero_first_arg(ptr %ap) { declare void @zero_first_arg(ptr, ptr) #5 [email protected] = constant [3 x i8] c"%r\00" [email protected]_float = constant [16 x i8] c"%% %1$+#0*.*hlf\00" [email protected] = global ptr null + +;; Both "float" and "fixed" aspects are present. The format string is "%r" (fixed-point), +;; so the "float" aspect is optimized away, and a reference to the needed "fixed" aspect is emitted. +define void @test_fixed_needed(i32 %arg) { +; CHECK-LABEL: @test_fixed_needed( +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.fixed, i32 [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(metadata !"basic_impl_fixed") +; CHECK-NEXT: ret void +; + call void (ptr, ...) @fixed_needed(ptr @.str.fixed, i32 %arg) + ret void +} + +declare void @fixed_needed(ptr, ...) #6 + +;; Both "float" and "fixed" aspects are present. The format string is "%f" (floating-point), +;; so the "fixed" aspect is optimized away, and a reference to the needed "float" aspect is emitted. +define void @test_float_needed(double %arg) { +; CHECK-LABEL: @test_float_needed( +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.float, double [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(metadata !"basic_impl_float") +; CHECK-NEXT: ret void +; + call void (ptr, ...) @float_needed(ptr @.str.float, double %arg) + ret void +} + +declare void @float_needed(ptr, ...) #6 + +;; Both "float" and "fixed" aspects are present. The format string is dynamic (not constant), +;; so both aspects are conservatively assumed to be needed, and no transformation occurs. +define void @test_dynamic_format(double %arg) { +; CHECK-LABEL: @test_dynamic_format( +; CHECK-NEXT: [[FMT:%.*]] = load ptr, ptr @.str.dynamic, align 4 +; CHECK-NEXT: call void (ptr, ...) @fixed_needed(ptr [[FMT]], double [[ARG:%.*]]) +; CHECK-NEXT: ret void +; + %fmt = load ptr, ptr @.str.dynamic, align 4 + call void (ptr, ...) @fixed_needed(ptr %fmt, double %arg) + ret void +} + +;; Both "float" and "fixed" aspects are present. The format string is dynamic (not constant), +;; but there are no floating-point arguments passed (only an integer). The "float" aspect is +;; optimized away, and only a reference to the needed "fixed" aspect is emitted. +define void @test_dynamic_format_no_float(i32 %arg) { +; CHECK-LABEL: @test_dynamic_format_no_float( +; CHECK-NEXT: [[FMT:%.*]] = load ptr, ptr @.str.dynamic, align 4 +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr [[FMT]], i32 [[ARG:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(metadata !"basic_impl_fixed") +; CHECK-NEXT: ret void +; + %fmt = load ptr, ptr @.str.dynamic, align 4 + call void (ptr, ...) @fixed_needed(ptr %fmt, i32 %arg) + ret void +} + +;; The format string contains a complex float specifier (testing escaped %, positional args, +;; flags, width, precision, length modifiers) but no fixed specifier. +;; The "fixed" aspect is optimized away, and only a reference to the needed "float" aspect is emitted. +define void @test_complex_format(i32 %width, i32 %prec, double %val) { +; CHECK-LABEL: @test_complex_format( +; CHECK-NEXT: call void (ptr, ...) @multiple_aspects_mod(ptr nonnull @.str.complex_float, i32 [[WIDTH:%.*]], i32 [[PREC:%.*]], double [[VAL:%.*]]) +; CHECK-NEXT: call void @llvm.reloc.none(metadata !"basic_impl_float") +; CHECK-NEXT: ret void +; + call void (ptr, ...) @fixed_needed(ptr @.str.complex_float, i32 %width, i32 %prec, double %val) + ret void +} + 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" } attributes #5 = { "modular-format"="printf,1,0,zero_first_arg_mod,basic_impl,float" } +attributes #6 = { "modular-format"="printf,1,2,multiple_aspects_mod,basic_impl,float,fixed" } diff --git a/llvm/test/Verifier/modular-format.ll b/llvm/test/Verifier/modular-format.ll index 4a9a5a08324ff..cdf1dd60d2772 100644 --- a/llvm/test/Verifier/modular-format.ll +++ b/llvm/test/Verifier/modular-format.ll @@ -39,3 +39,33 @@ define void @test_first_arg_index_in_bounds_varargs(i32 %arg, ...) "modular-form define void @test_first_arg_index_zero(i32 %arg) "modular-format"="printf,1,0,basic_mod,basic_impl" { ret void } + +define void @test_format_string_index_not_integer(i32 %arg, ...) "modular-format"="printf,foo,2,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute format string index is not an integer +; CHECK-NEXT: ptr @test_format_string_index_not_integer + +define void @test_format_string_index_zero(i32 %arg, ...) "modular-format"="printf,0,2,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute format string index must be greater than 0 +; CHECK-NEXT: ptr @test_format_string_index_zero + +define void @test_format_string_index_out_of_bounds(i32 %arg) "modular-format"="printf,2,1,basic_mod,basic_impl" { + ret void +} +; CHECK: modular-format attribute format string index is out of bounds +; CHECK-NEXT: ptr @test_format_string_index_out_of_bounds + +define void @test_modular_impl_fn_empty(i32 %arg) "modular-format"="printf,1,1,,basic_impl" { + ret void +} +; CHECK: modular-format attribute modular implementation function name cannot be empty +; CHECK-NEXT: ptr @test_modular_impl_fn_empty + +define void @test_impl_name_empty(i32 %arg) "modular-format"="printf,1,1,basic_mod," { + ret void +} +; CHECK: modular-format attribute implementation name cannot be empty +; CHECK-NEXT: ptr @test_impl_name_empty _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
