https://github.com/adams381 created https://github.com/llvm/llvm-project/pull/214986
A function declaration carried no CPU or feature attributes. CIRGen set them from `setNonAliasAttributes`, which runs only for a definition, where classic CodeGen sets them from `ConstructAttributeList` for a declaration too. We now set them properly in `constructAttributeList` alongside the other non-call-site attributes. Recording them on a declaration exposes a second bug. When a function is declared first and defined later with a `target` attribute, `setNonAliasAttributes` wrote the definition's values over the declaration's rather than replacing them, so a `tune-cpu` that the `target` attribute suppresses survived. It now clears the three attributes before writing, which is safe because `getCPUAndFeaturesAttributes` resolves the most recent declaration, so its result supersedes anything an earlier one wrote. Assisted-by: Cursor / claude-opus-5 >From fcd745d6b0c9a47f2145fe2444f53fafe2d1b528 Mon Sep 17 00:00:00 2001 From: Adam Smith <[email protected]> Date: Sat, 8 Aug 2026 09:27:36 -0700 Subject: [PATCH] [CIR] Record target-cpu and target-features on function declarations A function declaration carried no CPU or feature attributes. CIRGen set them from setNonAliasAttributes, which runs only for a definition, where classic CodeGen sets them from ConstructAttributeList for a declaration too. We now set them properly in constructAttributeList alongside the other non-call-site attributes. Recording them on a declaration exposes a second bug. When a function is declared first and defined later with a target attribute, setNonAliasAttributes wrote the definition's values over the declaration's rather than replacing them, so a tune-cpu that the target attribute suppresses survived. It now clears the three attributes before writing, which is safe because getCPUAndFeaturesAttributes resolves the most recent declaration, so its result supersedes anything an earlier one wrote. Assisted-by: Cursor / claude-opus-5 --- clang/lib/CIR/CodeGen/CIRGenCall.cpp | 9 ++++++- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 25 +++++++++++++----- clang/test/CIR/CodeGen/alloc-size.c | 4 +-- clang/test/CIR/CodeGen/asm-label-redirect.c | 4 +-- clang/test/CIR/CodeGen/attr-target-x86.c | 29 +++++++++++++++++++++ clang/test/CIR/CodeGen/global-init.cpp | 4 ++- 6 files changed, 63 insertions(+), 12 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp index 28670cf31694a..0fd1fc4deca51 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp @@ -492,11 +492,18 @@ void CIRGenModule::constructAttributeList( "sigsetjmp", "__sigsetjmp", "savectx", "getcontext"}; if (returnsTwiceFn.contains(name)) addUnitAttr(cir::CIRDialect::getReturnsTwiceAttrName()); + + llvm::StringMap<std::string> cpuAndFeatures; + if (getCPUAndFeaturesAttributes(calleeInfo.getCalleeDecl(), + cpuAndFeatures)) { + for (const auto &[key, val] : cpuAndFeatures) + attrs.set(key, builder.getStringAttr(val)); + } } // TODO(cir): A bunch of non-call-site function IR attributes from // declaration-specific information, including tail calls, - // cmse_nonsecure_entry, CPU-features/overrides, and hotpatch support. + // cmse_nonsecure_entry, and hotpatch support. // TODO(cir): Add loader-replaceable attribute here. diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index ae9cad0b7c30f..1b5ed527497fd 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -806,6 +806,13 @@ getFeatureDeltaFromDefault(const CIRGenModule &cgm, llvm::StringRef targetCPU, return delta; } +/// The names getCPUAndFeaturesAttributes produces. setNonAliasAttributes +/// clears them before writing a definition's values, so the two have to agree. +static constexpr llvm::StringLiteral targetCPUAttrName = "cir.target-cpu"; +static constexpr llvm::StringLiteral tuneCPUAttrName = "cir.tune-cpu"; +static constexpr llvm::StringLiteral targetFeaturesAttrName = + "cir.target-features"; + bool CIRGenModule::getCPUAndFeaturesAttributes( GlobalDecl gd, llvm::StringMap<std::string> &attrs, bool setTargetFeatures) { @@ -885,11 +892,11 @@ bool CIRGenModule::getCPUAndFeaturesAttributes( } if (!targetCPU.empty()) { - attrs["cir.target-cpu"] = targetCPU.str(); + attrs[targetCPUAttrName] = targetCPU.str(); addedAttr = true; } if (!tuneCPU.empty()) { - attrs["cir.tune-cpu"] = tuneCPU.str(); + attrs[tuneCPUAttrName] = tuneCPU.str(); addedAttr = true; } if (!features.empty() && setTargetFeatures) { @@ -899,7 +906,7 @@ bool CIRGenModule::getCPUAndFeaturesAttributes( return getTarget().isReadOnlyFeature(f.substr(1)); }); llvm::sort(features); - attrs["cir.target-features"] = llvm::join(features, ","); + attrs[targetFeaturesAttrName] = llvm::join(features, ","); addedAttr = true; } // TODO(cir): add metadata for AArch64 Function Multi Versioning. @@ -921,9 +928,15 @@ void CIRGenModule::setNonAliasAttributes(GlobalDecl gd, mlir::Operation *op) { if (auto func = dyn_cast<cir::FuncOp>(op)) { llvm::StringMap<std::string> attrs; if (getCPUAndFeaturesAttributes(gd, attrs)) { - // TODO(cir): Classic codegen removes the existing target-cpu, - // target-features, tune-cpu and fmv-features attributes here - // before adding the new ones. + // TODO(cir): Classic codegen also removes fmv-features here, which + // CIR does not emit yet. + // + // getCPUAndFeaturesAttributes reads the most recent declaration, so + // its result supersedes anything an earlier one wrote. Clear first: + // setAttr alone would leave a name this call no longer produces. + for (llvm::StringRef name : + {targetCPUAttrName, tuneCPUAttrName, targetFeaturesAttrName}) + func->removeAttr(name); for (const auto &[key, val] : attrs) func->setAttr(key, builder.getStringAttr(val)); } diff --git a/clang/test/CIR/CodeGen/alloc-size.c b/clang/test/CIR/CodeGen/alloc-size.c index e3ff12da1a083..c3c28cafb92db 100644 --- a/clang/test/CIR/CodeGen/alloc-size.c +++ b/clang/test/CIR/CodeGen/alloc-size.c @@ -9,9 +9,9 @@ typedef unsigned long size_t; -// CIR: cir.func{{.*}}@my_malloc(!s32i {llvm.noundef}){{.*}} attributes {allocsize = array<i32: 0>} +// CIR: cir.func{{.*}}@my_malloc(!s32i {llvm.noundef}){{.*}} attributes {allocsize = array<i32: 0>{{[,}]}} extern void *my_malloc(int) __attribute__((alloc_size(1))); -// CIR: cir.func{{.*}}@my_calloc(!s32i {llvm.noundef}, !s32i {llvm.noundef}){{.*}} attributes {allocsize = array<i32: 0, 1>} +// CIR: cir.func{{.*}}@my_calloc(!s32i {llvm.noundef}, !s32i {llvm.noundef}){{.*}} attributes {allocsize = array<i32: 0, 1>{{[,}]}} extern void *my_calloc(int, int) __attribute__((alloc_size(1, 2))); // CIR-LABEL: @call_direct diff --git a/clang/test/CIR/CodeGen/asm-label-redirect.c b/clang/test/CIR/CodeGen/asm-label-redirect.c index 3e40976df04a9..0dc01c912da2d 100644 --- a/clang/test/CIR/CodeGen/asm-label-redirect.c +++ b/clang/test/CIR/CodeGen/asm-label-redirect.c @@ -26,8 +26,8 @@ int test(const char *p) { // it sees first - here, the my_stat declaration. // // CIR-LABEL: cir.func private @real_impl( -// CIR-SAME: !cir.ptr<!s8i> {{.*}}, -// CIR-SAME: !cir.ptr<!rec_my_stat> {{.*}}) -> !s32i +// CIR-SAME: !cir.ptr<!s8i> {{[^,]*}}, +// CIR-SAME: !cir.ptr<!rec_my_stat> {{[^,]*}}) -> !s32i // CIR-LABEL: cir.func {{.*}} @test( // diff --git a/clang/test/CIR/CodeGen/attr-target-x86.c b/clang/test/CIR/CodeGen/attr-target-x86.c index c99484c42a1db..d46749c58ac34 100644 --- a/clang/test/CIR/CodeGen/attr-target-x86.c +++ b/clang/test/CIR/CodeGen/attr-target-x86.c @@ -26,6 +26,8 @@ // LLVM: define {{.*}}@f_avx10_1{{.*}} [[f_avx10_1:#[0-9]+]] // LLVM: define {{.*}}@f_prefer_256_bit({{.*}} [[f_prefer_256_bit:#[0-9]+]] // LLVM: define {{.*}}@f_no_prefer_256_bit({{.*}} [[f_no_prefer_256_bit:#[0-9]+]] +// LLVM: declare {{.*}}@f_decl_only() [[f_decl_only:#[0-9]+]] +// LLVM: declare {{.*}}@f_decl_default() [[f_decl_default:#[0-9]+]] // CIR: cir.func{{.*}} @f_default() // CIR-SAME: "cir.target-cpu" = "i686" @@ -124,6 +126,11 @@ void usage(void){ // f_use_before_def: same attributes as f_lakemont_mmx (checked above) - the // definition's attribute should be propagated to the earlier declaration. +// The dictionary closes after the features, so the tune-cpu the declaration +// recorded before the definition was seen does not survive. +// CIR: cir.func{{.*}} @f_use_before_def() +// CIR-SAME: "cir.target-cpu" = "lakemont" +// CIR-SAME: "cir.target-features" = "+cx8,+mmx", nothrow} __attribute__((target("arch=lakemont,mmx"))) void f_use_before_def(void) {} @@ -184,3 +191,25 @@ void f_prefer_256_bit(void) {} // LLVM: [[f_no_prefer_256_bit]] = {{.*}}"target-features"="{{.*}}-prefer-256-bit __attribute__((target("no-prefer-256-bit"))) void f_no_prefer_256_bit(void) {} + +// A declaration gets the attributes too. target(arch=) suppresses tune-cpu, +// so the dictionary closes after the features. +// CIR: cir.func private @f_decl_only() +// CIR-SAME: "cir.target-cpu" = "lakemont" +// CIR-SAME: "cir.target-features" = "+cx8,+mmx"} + +// LLVM: [[f_decl_only]] = {{.*}}"target-cpu"="lakemont" "target-features"="+cx8,+mmx" } +__attribute__((target("arch=lakemont,mmx"))) +void f_decl_only(void); +void use_decl_only(void) { f_decl_only(); } + +// A declaration with no target attribute takes all three from the command +// line, tune-cpu included. +// CIR: cir.func private @f_decl_default() +// CIR-SAME: "cir.target-cpu" = "i686" +// CIR-SAME: "cir.target-features" = "+cmov,+cx8,+x87" +// CIR-SAME: "cir.tune-cpu" = "i686" + +// LLVM: [[f_decl_default]] = {{.*}}"target-cpu"="i686" "target-features"="+cmov,+cx8,+x87" "tune-cpu"="i686" +void f_decl_default(void); +void use_decl_default(void) { f_decl_default(); } diff --git a/clang/test/CIR/CodeGen/global-init.cpp b/clang/test/CIR/CodeGen/global-init.cpp index 7abb70e42d608..c5bdded22af98 100644 --- a/clang/test/CIR/CodeGen/global-init.cpp +++ b/clang/test/CIR/CodeGen/global-init.cpp @@ -230,7 +230,7 @@ ArrayDtor arrDtor[16]; // LLVM: %[[CUR:.*]] = load ptr, ptr %[[CUR_ADDR]] // LLVM: %[[PREV:.*]] = getelementptr %struct.ArrayDtor, ptr %[[CUR]], i64 -1 // LLVM: store ptr %[[PREV]], ptr %[[CUR_ADDR]] -// LLVM: call void @_ZN9ArrayDtorD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %[[PREV]]) #0 +// LLVM: call void @_ZN9ArrayDtorD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %[[PREV]]) [[NOUNWIND:#[0-9]+]] // LLVM: br label %[[LOOP_COND]] // LLVM: [[LOOP_END]]: // LLVM: ret void @@ -279,6 +279,8 @@ ArrayDtor arrDtor[16]; // LLVM: call void @__cxx_global_var_init.4() // LLVM: call void @__cxx_global_var_init.5() +// LLVM: attributes [[NOUNWIND]] = { nounwind } + // OGCG: define internal void @_GLOBAL__sub_I_[[FILENAME]]() {{.*}} section ".text.startup" { // OGCG: call void @__cxx_global_var_init() // OGCG: call void @__cxx_global_var_init.1() _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
