llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Vladimir Miloserdov (miloserdow) <details> <summary>Changes</summary> Implement the definition-time precedence logic in setCIRFunctionAttributesForDefinition(). This fills the gap where constructAttributeList() already sets cold/hot/noduplicate as discardable attrs, but the definition-time precedence for optnone (and its interactions with optsize/minsize) was missing. --- Full diff: https://github.com/llvm/llvm-project/pull/184588.diff 6 Files Affected: - (modified) clang/include/clang/CIR/Dialect/IR/CIRDialect.td (+1) - (modified) clang/include/clang/CIR/MissingFeatures.h (-4) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+39-6) - (added) clang/test/CIR/CodeGen/func-attrs.cpp (+94) - (modified) clang/test/CIR/CodeGen/optsize-func-attr.cpp (+9-6) - (added) clang/test/CIR/Lowering/func-attrs.cir (+35) ``````````diff diff --git a/clang/include/clang/CIR/Dialect/IR/CIRDialect.td b/clang/include/clang/CIR/Dialect/IR/CIRDialect.td index 665912419d0d4..4a479e5b3271e 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRDialect.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRDialect.td @@ -54,6 +54,7 @@ def CIR_Dialect : Dialect { static llvm::StringRef getNoCallerSavedRegsAttrName() { return "no_caller_saved_registers"; } static llvm::StringRef getNoCallbackAttrName() { return "nocallback"; } static llvm::StringRef getAllocSizeAttrName() { return "allocsize"; } + static llvm::StringRef getOptimizeNoneAttrName() { return "optimize_none"; } static llvm::StringRef getOptimizeForSizeAttrName() { return "optsize"; } static llvm::StringRef getMinSizeAttrName() { return "minsize"; } // Note: we have to name this with the underscore instead of the dash like diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index d206503d914f5..f593218ad89c3 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -72,19 +72,15 @@ struct MissingFeatures { static bool opFuncArmStreamingAttr() { return false; } static bool opFuncAstDeclAttr() { return false; } static bool opFuncCallingConv() { return false; } - static bool opFuncColdHotAttr() { return false; } static bool opFuncCPUAndFeaturesAttributes() { return false; } static bool opFuncExceptions() { return false; } static bool opFuncExtraAttrs() { return false; } static bool opFuncMaybeHandleStaticInExternC() { return false; } - static bool opFuncMinSizeAttr() { return false; } static bool opFuncMultipleReturnVals() { return false; } static bool opFuncNakedAttr() { return false; } - static bool opFuncNoDuplicateAttr() { return false; } static bool opFuncNoUnwind() { return false; } static bool opFuncOpenCLKernelMetadata() { return false; } static bool opFuncOperandBundles() { return false; } - static bool opFuncOptNoneAttr() { return false; } static bool opFuncParameterAttributes() { return false; } static bool opFuncReadOnly() { return false; } static bool opFuncSection() { return false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 3ef487465ab80..d72e0a67d1b93 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -2325,14 +2325,34 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition( assert(!cir::MissingFeatures::opFuncArmStreamingAttr()); assert(!cir::MissingFeatures::opFuncArmNewAttr()); - assert(!cir::MissingFeatures::opFuncOptNoneAttr()); - assert(!cir::MissingFeatures::opFuncMinSizeAttr()); assert(!cir::MissingFeatures::opFuncNakedAttr()); - assert(!cir::MissingFeatures::opFuncNoDuplicateAttr()); assert(!cir::MissingFeatures::hlsl()); - // Handle inline attributes - if (decl->hasAttr<NoInlineAttr>() && !isAlwaysInline) { + // Track whether we need to add the optnone attribute, starting with the + // default for this optimization level. + bool shouldAddOptNone = + !codeGenOpts.DisableO0ImplyOptNone && codeGenOpts.OptimizationLevel == 0; + // We can't add optnone in the following cases, it won't pass the verifier. + shouldAddOptNone &= !decl->hasAttr<MinSizeAttr>(); + shouldAddOptNone &= !decl->hasAttr<AlwaysInlineAttr>(); + + if ((shouldAddOptNone || decl->hasAttr<OptimizeNoneAttr>()) && + !isAlwaysInline) { + // Add optnone, but do so only if the function isn't always_inline. + f->setAttr(cir::CIRDialect::getOptimizeNoneAttrName(), + mlir::UnitAttr::get(&getMLIRContext())); + + // OptimizeNone implies noinline; we should not be inlining such functions. + f.setInlineKind(cir::InlineKind::NoInline); + + // OptimizeNone wins over OptimizeForSize and MinSize. + f->removeAttr(cir::CIRDialect::getOptimizeForSizeAttrName()); + f->removeAttr(cir::CIRDialect::getMinSizeAttrName()); + } else if (decl->hasAttr<NoDuplicateAttr>()) { + // NoDuplicate is already set as a discardable attr by + // constructAttributeList. No additional action needed here beyond + // occupying the correct position in the precedence chain. + } else if (decl->hasAttr<NoInlineAttr>() && !isAlwaysInline) { // Add noinline if the function isn't always_inline. f.setInlineKind(cir::InlineKind::NoInline); } else if (decl->hasAttr<AlwaysInlineAttr>() && !isNoInline) { @@ -2373,7 +2393,20 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition( } } - assert(!cir::MissingFeatures::opFuncColdHotAttr()); + // Add other optimization-related attributes if we are optimizing this + // function (i.e. OptimizeNone is not present). + if (!decl->hasAttr<OptimizeNoneAttr>()) { + if (decl->hasAttr<ColdAttr>()) { + // ColdAttr implies OptimizeForSize, but not if we're already adding + // optnone (which would remove it anyway). + if (!shouldAddOptNone) + f->setAttr(cir::CIRDialect::getOptimizeForSizeAttrName(), + mlir::UnitAttr::get(&getMLIRContext())); + } + if (decl->hasAttr<MinSizeAttr>()) + f->setAttr(cir::CIRDialect::getMinSizeAttrName(), + mlir::UnitAttr::get(&getMLIRContext())); + } } cir::FuncOp CIRGenModule::getOrCreateCIRFunction( diff --git a/clang/test/CIR/CodeGen/func-attrs.cpp b/clang/test/CIR/CodeGen/func-attrs.cpp new file mode 100644 index 0000000000000..414f927719d52 --- /dev/null +++ b/clang/test/CIR/CodeGen/func-attrs.cpp @@ -0,0 +1,94 @@ +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -O1 -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -O1 -fclangir -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -O1 -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s +// +// Test -O0 implicit optnone: +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -O0 -fclangir -emit-cir %s -o %t-o0.cir +// RUN: FileCheck --check-prefix=O0CIR --input-file=%t-o0.cir %s +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -O0 -fclangir -emit-llvm %s -o %t-o0.ll +// RUN: FileCheck --check-prefix=O0LLVM --input-file=%t-o0.ll %s + +extern "C" { + +// CIR: cir.func no_inline dso_local @optnone_func() +// CIR-SAME: attributes {optimize_none} +// LLVM: define dso_local void @optnone_func() {{.*}}#[[OPTNONE_ATTR:[0-9]+]] +// OGCG: define dso_local void @optnone_func() {{.*}}#[[OPTNONE_ATTR:[0-9]+]] +__attribute__((optnone)) +void optnone_func() {} + +// CIR: cir.func dso_local @cold_func() +// CIR-SAME: attributes {cold, optsize} +// LLVM: define dso_local void @cold_func() {{.*}}#[[COLD_ATTR:[0-9]+]] +// OGCG: define dso_local void @cold_func() {{.*}}#[[COLD_ATTR:[0-9]+]] +__attribute__((cold)) +void cold_func() {} + +// CIR: cir.func dso_local @hot_func() +// CIR-SAME: attributes {hot} +// LLVM: define dso_local void @hot_func() {{.*}}#[[HOT_ATTR:[0-9]+]] +// OGCG: define dso_local void @hot_func() {{.*}}#[[HOT_ATTR:[0-9]+]] +__attribute__((hot)) +void hot_func() {} + +// CIR: cir.func dso_local @noduplicate_func() +// CIR-SAME: attributes {noduplicate} +// LLVM: define dso_local void @noduplicate_func() {{.*}}#[[ND_ATTR:[0-9]+]] +// OGCG: define dso_local void @noduplicate_func() {{.*}}#[[ND_ATTR:[0-9]+]] +__attribute__((noduplicate)) +void noduplicate_func() {} + +// CIR: cir.func dso_local @minsize_func() +// CIR-SAME: attributes {minsize} +// LLVM: define dso_local void @minsize_func() {{.*}}#[[MINSIZE_ATTR:[0-9]+]] +// OGCG: define dso_local void @minsize_func() {{.*}}#[[MINSIZE_ATTR:[0-9]+]] +__attribute__((minsize)) +void minsize_func() {} + +// optnone + cold: optnone wins, cold stays but optsize must NOT be added. +// CIR: cir.func no_inline dso_local @optnone_cold_func() +// CIR-SAME: attributes {cold, optimize_none} +// CIR-NOT: optsize +// LLVM: define dso_local void @optnone_cold_func() {{.*}}#[[OPTNONE_COLD_ATTR:[0-9]+]] +// OGCG: define dso_local void @optnone_cold_func() {{.*}}#[[OPTNONE_COLD_ATTR:[0-9]+]] +__attribute__((optnone, cold)) +void optnone_cold_func() {} + +// optnone + hot: hot stays (set by constructAttributeList), optnone+noinline added. +// CIR: cir.func no_inline dso_local @optnone_hot_func() +// CIR-SAME: attributes {hot, optimize_none} +// LLVM: define dso_local void @optnone_hot_func() {{.*}}#[[OPTNONE_HOT_ATTR:[0-9]+]] +// OGCG: define dso_local void @optnone_hot_func() {{.*}}#[[OPTNONE_HOT_ATTR:[0-9]+]] +__attribute__((optnone, hot)) +void optnone_hot_func() {} + +// -O0 implicit optnone: normal functions get optimize_none + noinline. +// O0CIR: cir.func no_inline dso_local @optnone_func() +// O0CIR-SAME: optimize_none +// O0CIR: cir.func no_inline dso_local @cold_func() +// O0CIR-SAME: optimize_none +// minsize suppresses implicit optnone at -O0. +// O0CIR: cir.func no_inline dso_local @minsize_func() attributes {minsize} { +// O0LLVM: define dso_local void @optnone_func() {{.*}}#[[O0_OPTNONE:[0-9]+]] +// O0LLVM-DAG: attributes #[[O0_OPTNONE]] = {{{.*}}noinline{{.*}}optnone{{.*}}} + +} + +// LLVM-DAG: attributes #[[OPTNONE_ATTR]] = {{{.*}}noinline{{.*}}optnone{{.*}}} +// LLVM-DAG: attributes #[[COLD_ATTR]] = {{{.*}}cold{{.*}}optsize{{.*}}} +// LLVM-DAG: attributes #[[HOT_ATTR]] = {{{.*}}hot{{.*}}} +// LLVM-DAG: attributes #[[ND_ATTR]] = {{{.*}}noduplicate{{.*}}} +// LLVM-DAG: attributes #[[MINSIZE_ATTR]] = {{{.*}}minsize{{.*}}} +// LLVM-DAG: attributes #[[OPTNONE_COLD_ATTR]] = {{{.*}}cold{{.*}}noinline{{.*}}optnone{{.*}}} +// LLVM-DAG: attributes #[[OPTNONE_HOT_ATTR]] = {{{.*}}hot{{.*}}noinline{{.*}}optnone{{.*}}} + +// OGCG-DAG: attributes #[[OPTNONE_ATTR]] = {{{.*}}noinline{{.*}}optnone{{.*}}} +// OGCG-DAG: attributes #[[COLD_ATTR]] = {{{.*}}cold{{.*}}optsize{{.*}}} +// OGCG-DAG: attributes #[[HOT_ATTR]] = {{{.*}}hot{{.*}}} +// OGCG-DAG: attributes #[[ND_ATTR]] = {{{.*}}noduplicate{{.*}}} +// OGCG-DAG: attributes #[[MINSIZE_ATTR]] = {{{.*}}minsize{{.*}}} +// OGCG-DAG: attributes #[[OPTNONE_COLD_ATTR]] = {{{.*}}cold{{.*}}noinline{{.*}}optnone{{.*}}} +// OGCG-DAG: attributes #[[OPTNONE_HOT_ATTR]] = {{{.*}}hot{{.*}}noinline{{.*}}optnone{{.*}}} diff --git a/clang/test/CIR/CodeGen/optsize-func-attr.cpp b/clang/test/CIR/CodeGen/optsize-func-attr.cpp index 28441b8558584..2c7c009241fff 100644 --- a/clang/test/CIR/CodeGen/optsize-func-attr.cpp +++ b/clang/test/CIR/CodeGen/optsize-func-attr.cpp @@ -40,7 +40,7 @@ extern "C" { // CIR: cir.call{{.*}}@optnone() // CIR-NOT: optsize // CIR-NOT: minsize - // LLVM: call void @optnone() #[[OPTNONE_ATTR]] + // LLVM: call void @optnone() #[[OPTNONE_CALL_ATTR:.*]] // OGCG: call void @optnone() #[[OPTNONE_CALL_ATTR:.*]] // CIR: cir.return @@ -58,10 +58,13 @@ extern "C" { // attributes for caller, to block the 'NOT'. // BOTH: attributes // -// CIR doesn't have sufficiently different 'attributes' implemented for the -// caller and the callee to be different when doing -O settings (as 'optnone' -// is the only difference). So the below call attributes are only necessary -// for classic codegen. +// Call-site attributes differ from definition attributes because optnone, +// noinline, etc. are definition-only. Verify the call attrs separately. +// LLVM: attributes #[[OPTNONE_CALL_ATTR]] +// LLVM-NOT: optsize +// LLVM-NOT: minsize +// LLVM: llvm.module.flags +// // OGCG: attributes #[[NORMAL_CALL_ATTR]] // OGCGOZ-SAME: minsize // OGCG-SAME: optsize @@ -71,4 +74,4 @@ extern "C" { // OGCG-NOT: minsize // // to block the 'NOT'. -// BOTH: llvm.module.flags +// OGCG: llvm.module.flags diff --git a/clang/test/CIR/Lowering/func-attrs.cir b/clang/test/CIR/Lowering/func-attrs.cir new file mode 100644 index 0000000000000..d9bf2fc6a8eb5 --- /dev/null +++ b/clang/test/CIR/Lowering/func-attrs.cir @@ -0,0 +1,35 @@ +// RUN: cir-opt %s --cir-to-llvm -o - | FileCheck %s + +module { + // CHECK: llvm.func @optimize_none_func() attributes {no_inline, optimize_none} + cir.func no_inline @optimize_none_func() attributes {optimize_none} { + cir.return + } + + // CHECK: llvm.func @cold_func() attributes {cold, optsize} + cir.func @cold_func() attributes {cold, optsize} { + cir.return + } + + // CHECK: llvm.func @hot_func() attributes {hot} + cir.func @hot_func() attributes {hot} { + cir.return + } + + // CHECK: llvm.func @noduplicate_func() attributes {noduplicate} + cir.func @noduplicate_func() attributes {noduplicate} { + cir.return + } + + // CHECK: llvm.func @minsize_func() attributes {minsize} + cir.func @minsize_func() attributes {minsize} { + cir.return + } + + // optnone + cold: optimize_none is set, cold is preserved, but no optsize. + // CHECK: llvm.func @optnone_cold_func() attributes {cold, no_inline, optimize_none} + // CHECK-NOT: optsize + cir.func no_inline @optnone_cold_func() attributes {cold, optimize_none} { + cir.return + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/184588 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
