llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Kaviya Rajendiran (kaviya2510) <details> <summary>Changes</summary> The option `-funique-internal-linkage-names` appends an MD5 hash suffix `(.__uniq.<hash>)` to internal procedure names and sets `"sample-profile-suffix-elision-policy"="selected"` on those functions, enabling the sample profiler to correctly match profiles to internal procedures across compilation units. Implementation: - Added a unique hash suffix to internal procedure names when `-funique-internal-linkage-names` is enabled. The suffix is derived from the source file path and disambiguates identically named internal procedures across different compilation units, which helps accurate sample-based profiling. - Added the function attribute `"sample-profile-suffix-elision-policy"="selected"` on internal procedures in LLVM IR. This attribute is used by the LLVM sample profile loader to control suffix stripping during profile matching --- Full diff: https://github.com/llvm/llvm-project/pull/216680.diff 18 Files Affected: - (modified) clang/include/clang/Options/Options.td (+2-2) - (modified) clang/lib/Driver/ToolChains/Flang.cpp (+2) - (modified) flang/include/flang/Frontend/CodeGenOptions.def (+1) - (modified) flang/include/flang/Lower/Bridge.h (+4) - (modified) flang/include/flang/Optimizer/Transforms/Passes.td (+4) - (modified) flang/include/flang/Tools/CrossToolHelpers.h (+3) - (modified) flang/lib/Frontend/CompilerInvocation.cpp (+5) - (modified) flang/lib/Lower/Bridge.cpp (+14-1) - (modified) flang/lib/Optimizer/Passes/Pipelines.cpp (+1) - (modified) flang/lib/Optimizer/Transforms/FunctionAttr.cpp (+7) - (added) flang/test/Driver/funique-internal-linkage-names.f90 (+20) - (added) flang/test/Integration/unique-internal-linkage-names.f90 (+22) - (added) flang/test/Transforms/function-attrs-unique-internal-linkage-names.fir (+26) - (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+2-1) - (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+7) - (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+5) - (added) mlir/test/Target/LLVMIR/Import/sample-profile-suffix-elision-policy.ll (+15) - (added) mlir/test/Target/LLVMIR/sample-profile-suffix-elision-policy.mlir (+7) ``````````diff diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index adc4224dd561c..cc3745da52285 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -5051,10 +5051,10 @@ defm unique_basic_block_section_names : BoolFOption<"unique-basic-block-section- NegFlag<SetFalse>>; defm unique_internal_linkage_names : BoolFOption<"unique-internal-linkage-names", CodeGenOpts<"UniqueInternalLinkageNames">, DefaultFalse, - PosFlag<SetTrue, [], [ClangOption, CC1Option], + PosFlag<SetTrue, [], [ClangOption, CC1Option, FlangOption, FC1Option], "Uniqueify Internal Linkage Symbol Names by appending" " the MD5 hash of the module path">, - NegFlag<SetFalse>>; + NegFlag<SetFalse, [], [ClangOption, CC1Option, FlangOption, FC1Option]>>; defm unique_section_names : BoolFOption<"unique-section-names", CodeGenOpts<"UniqueSectionNames">, DefaultTrue, NegFlag<SetFalse, [], [ClangOption, CC1Option], diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index a48e41159f367..2b430f0c693d6 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -339,6 +339,8 @@ void Flang::addCodegenOptions(const ArgList &Args, options::OPT_fno_experimental_loop_fusion); Args.addOptInFlag(CmdArgs, options::OPT_ffp_sum_reassociation, options::OPT_fno_fp_sum_reassociation); + Args.addOptInFlag(CmdArgs, options::OPT_funique_internal_linkage_names, + options::OPT_fno_unique_internal_linkage_names); handleInterchangeLoopsArgs(Args, CmdArgs); handleVectorizeLoopsArgs(Args, CmdArgs); diff --git a/flang/include/flang/Frontend/CodeGenOptions.def b/flang/include/flang/Frontend/CodeGenOptions.def index d49a7f3647eec..0726b71a71dff 100644 --- a/flang/include/flang/Frontend/CodeGenOptions.def +++ b/flang/include/flang/Frontend/CodeGenOptions.def @@ -60,6 +60,7 @@ CODEGENOPT(AliasAnalysis, 1, 0) ///< Enable alias analysis pass CODEGENOPT(DwarfVersion, 3, 0) ///< Dwarf version CODEGENOPT(DebugInfoForProfiling, 1, 0) ///< Emit extra debug info to make sample profile more accurate. CODEGENOPT(PseudoProbeForProfiling, 1, 0) ///< Emit pseudo probes for sample profiling. +CODEGENOPT(UniqueInternalLinkageNames, 1, 0) ///< Append MD5 hash to internal linkage symbols. CODEGENOPT(Underscoring, 1, 1) ENUM_CODEGENOPT(FPMaxminBehavior, Fortran::common::FPMaxminBehavior, 2, Fortran::common::FPMaxminBehavior::Legacy) diff --git a/flang/include/flang/Lower/Bridge.h b/flang/include/flang/Lower/Bridge.h index dbddef7b1169d..f8421f27e6ec2 100644 --- a/flang/include/flang/Lower/Bridge.h +++ b/flang/include/flang/Lower/Bridge.h @@ -24,6 +24,7 @@ #include "flang/Support/Fortran.h" #include "mlir/IR/BuiltinOps.h" #include "mlir/IR/OwningOpRef.h" +#include "llvm/ProfileData/SampleProf.h" #include <set> namespace llvm { @@ -118,6 +119,8 @@ class LoweringBridge { return languageFeatures; } + const std::string &getModuleNameHash() const { return moduleNameHash; } + /// Create a folding context. Careful: this is very expensive. Fortran::evaluate::FoldingContext createFoldingContext(); @@ -180,6 +183,7 @@ class LoweringBridge { const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults; const Fortran::common::LanguageFeatureControl &languageFeatures; std::set<std::string> tempNames; + std::string moduleNameHash; std::optional<mlir::DiagnosticEngine::HandlerID> diagHandlerID; }; diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td index e7bb8ae9bb9bf..7d752c88af8b8 100644 --- a/flang/include/flang/Optimizer/Transforms/Passes.td +++ b/flang/include/flang/Optimizer/Transforms/Passes.td @@ -489,6 +489,10 @@ def FunctionAttr : Pass<"function-attr", "mlir::func::FuncOp"> { /*default=*/"false", "Set the use-sample-profile attribute on functions in the " "module.">, + Option<"UniqueInternalLinkageNames", "unique-internal-linkage-names", + "bool", /*default=*/"false", + "Set the sample-profile-suffix-elision-policy attribute on " + "internal linkage functions in the module.">, Option<"tuneCPU", "tune-cpu", "std::string", /*default=*/"", "Set the tune-cpu attribute on functions in the module.">, Option<"setNoCapture", "set-nocapture", "bool", /*default=*/"false", diff --git a/flang/include/flang/Tools/CrossToolHelpers.h b/flang/include/flang/Tools/CrossToolHelpers.h index fb8007637b114..1db449e737968 100644 --- a/flang/include/flang/Tools/CrossToolHelpers.h +++ b/flang/include/flang/Tools/CrossToolHelpers.h @@ -138,6 +138,7 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks { Reciprocals = opts.Reciprocals; PreferVectorWidth = opts.PreferVectorWidth; UseSampleProfile = !opts.SampleProfileFile.empty(); + UniqueInternalLinkageNames = opts.UniqueInternalLinkageNames; DebugInfoForProfiling = opts.DebugInfoForProfiling; if (opts.InstrumentFunctions) { InstrumentFunctionEntry = "__cyg_profile_func_enter"; @@ -176,6 +177,8 @@ struct MLIRToLLVMPassPipelineConfig : public FlangEPCallBacks { bool EnableOpenMPIsTargetDevice = false; ///< Compiling for an OpenMP target device. bool UseSampleProfile = false; ///< Enable sample based profiling + bool UniqueInternalLinkageNames = false; ///< Append MD5 hash suffix to + ///< internal linkage symbol names. bool DebugInfoForProfiling = false; ///< Enable extra debugging info bool EnableOpenMPSimd = false; ///< Enable OpenMP simd-only mode. bool SkipConvertComplexPow = false; ///< Do not run complex pow conversion. diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp index b57bc4583be38..72d611e52ca10 100644 --- a/flang/lib/Frontend/CompilerInvocation.cpp +++ b/flang/lib/Frontend/CompilerInvocation.cpp @@ -316,6 +316,11 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts, clang::options::OPT_fno_safe_trampoline, false)) opts.EnableSafeTrampoline = 1; + if (args.hasFlag(clang::options::OPT_funique_internal_linkage_names, + clang::options::OPT_fno_unique_internal_linkage_names, + false)) + opts.UniqueInternalLinkageNames = 1; + if (args.hasFlag(clang::options::OPT_ffp_sum_reassociation, clang::options::OPT_fno_fp_sum_reassociation, false)) opts.SplitSumExpressionTree = 1; diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index bff6b51e50e18..6775fb91e55eb 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -1219,9 +1219,15 @@ class FirConverter : public Fortran::lower::AbstractConverter { } std::string mangleName(const Fortran::semantics::Symbol &symbol) override final { - return Fortran::lower::mangle::mangleName( + std::string mangledName = Fortran::lower::mangle::mangleName( symbol, scopeBlockIdMap, /*keepExternalInScope=*/false, getLoweringOptions().getUnderscoring()); + const auto &hash = bridge.getModuleNameHash(); + if (!hash.empty() && + Fortran::semantics::ClassifyProcedure(symbol) == + Fortran::semantics::ProcedureDefinitionClass::Internal) + mangledName += hash; + return mangledName; } std::string mangleName( const Fortran::semantics::DerivedTypeSpec &derivedType) override final { @@ -7004,6 +7010,13 @@ Fortran::lower::LoweringBridge::LoweringBridge( fir::setIsPIE(*module, cgOpts.IsPIE); if (cgOpts.RecordCommandLine) fir::setCommandline(*module, *cgOpts.RecordCommandLine); + + if (cgOpts.UniqueInternalLinkageNames) { + if (auto fileLoc = mlir::dyn_cast<mlir::FileLineColLoc>(module->getLoc())) { + moduleNameHash = + llvm::getUniqueInternalLinkagePostfix(fileLoc.getFilename()); + } + } } Fortran::lower::LoweringBridge::~LoweringBridge() { diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp index 15a342e10fc7f..e05d01e46a9b3 100644 --- a/flang/lib/Optimizer/Passes/Pipelines.cpp +++ b/flang/lib/Optimizer/Passes/Pipelines.cpp @@ -441,6 +441,7 @@ void createDefaultFIRCodeGenPassPipeline(mlir::PassManager &pm, config.InstrumentFunctionExit, config.NoInfsFPMath, config.NoNaNsFPMath, config.ApproxFuncFPMath, config.NoSignedZerosFPMath, config.UnsafeFPMath, config.Reciprocals, config.PreferVectorWidth, config.UseSampleProfile, + config.UniqueInternalLinkageNames, /*tuneCPU=*/"", setNoCapture, setNoAlias, setReadOnly})); if (config.EnableOpenMP) { diff --git a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp index 45b32d13ad62e..44acf6e3ef7ae 100644 --- a/flang/lib/Optimizer/Transforms/FunctionAttr.cpp +++ b/flang/lib/Optimizer/Transforms/FunctionAttr.cpp @@ -144,6 +144,13 @@ void FunctionAttrPass::runOnOperation() { context, mlir::LLVM::LLVMFuncOp::getUseSampleProfileAttrName( llvmFuncOpName)), mlir::BoolAttr::get(context, true)); + if (UniqueInternalLinkageNames && fir::isInternalProcedure(func)) + func->setAttr( + getLlvmFuncPropertyAttrName( + context, + mlir::LLVM::LLVMFuncOp::getSampleProfileSuffixElisionPolicyAttrName( + llvmFuncOpName)), + mlir::StringAttr::get(context, "selected")); LLVM_DEBUG(llvm::dbgs() << "=== End " DEBUG_TYPE " ===\n"); } diff --git a/flang/test/Driver/funique-internal-linkage-names.f90 b/flang/test/Driver/funique-internal-linkage-names.f90 new file mode 100644 index 0000000000000..88cafb520a019 --- /dev/null +++ b/flang/test/Driver/funique-internal-linkage-names.f90 @@ -0,0 +1,20 @@ +! Test that -funique-internal-linkage-names / -fno-unique-internal-linkage-names are forwarded to flang -fc1. + +! RUN: %flang -### %s 2>&1 | FileCheck %s --check-prefix=DEFAULT +! RUN: %flang -### -funique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=ENABLED +! RUN: %flang -### -fno-unique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=DISABLED +! RUN: %flang -### -funique-internal-linkage-names -fno-unique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=DISABLED +! RUN: %flang -### -fno-unique-internal-linkage-names -funique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=ENABLED + +! DEFAULT-NOT: "-funique-internal-linkage-names" +! DEFAULT-NOT: "-fno-unique-internal-linkage-names" + +! ENABLED: "-fc1"{{.*}}"-funique-internal-linkage-names" +! DISABLED-NOT: "-funique-internal-linkage-names" + +subroutine host() + call inner() +contains + subroutine inner() + end subroutine +end subroutine diff --git a/flang/test/Integration/unique-internal-linkage-names.f90 b/flang/test/Integration/unique-internal-linkage-names.f90 new file mode 100644 index 0000000000000..c96e854b8eae4 --- /dev/null +++ b/flang/test/Integration/unique-internal-linkage-names.f90 @@ -0,0 +1,22 @@ +! Test that -funique-internal-linkage-names appends a hash suffix to internal +! procedures and sets the "sample-profile-suffix-elision-policy" attribute. + +! RUN: %flang_fc1 -emit-llvm -funique-internal-linkage-names -o - %s | FileCheck %s + +! CHECK-LABEL: define void @test_( +! CHECK : call void @_QFtestPfooX__uniqX{{[0-9]+}}(ptr {{.*}}) + +! CHECK-LABEL: define internal void @_QFtestPfooX__uniqX{{[0-9]+}}(ptr {{.*}}) #0 +! CHECK-NOT : define internal void @_QFtestPfoo( + +! CHECK: attributes #0 = { "sample-profile-suffix-elision-policy"="selected" } + +subroutine test(x) + integer, intent(inout) :: x + call foo(x) +contains + subroutine foo(y) + integer, intent(inout) :: y + y = y + 1 + end subroutine +end subroutine diff --git a/flang/test/Transforms/function-attrs-unique-internal-linkage-names.fir b/flang/test/Transforms/function-attrs-unique-internal-linkage-names.fir new file mode 100644 index 0000000000000..9d5cf40ea6e35 --- /dev/null +++ b/flang/test/Transforms/function-attrs-unique-internal-linkage-names.fir @@ -0,0 +1,26 @@ +// RUN: fir-opt --function-attr="unique-internal-linkage-names=true" %s | FileCheck %s --check-prefix=ENABLED +// RUN: fir-opt --function-attr="unique-internal-linkage-names=false" %s | FileCheck %s --check-prefix=DISABLED + +// Internal procedure: has fir.host_symbol, should get the attribute. +// ENABLED-LABEL: func.func @_QFhost_subPinner( +// ENABLED-SAME: llvm.sample_profile_suffix_elision_policy = "selected" + +// DISABLED-LABEL: func.func @_QFhost_subPinner( +// DISABLED-NOT: sample_profile_suffix_elision_policy +func.func @_QFhost_subPinner(%arg0: !fir.ref<i32>) attributes {fir.host_symbol = @_QFhost_sub} { + return +} + +// Host procedure: no fir.host_symbol, should NOT get the attribute. +// ENABLED-LABEL: func.func @_QFhost_sub( +// ENABLED-NOT: sample_profile_suffix_elision_policy +func.func @_QFhost_sub(%arg0: !fir.ref<i32>) { + return +} + +// External procedure: should NOT get the attribute. +// ENABLED-LABEL: func.func @_QPexternal_sub( +// ENABLED-NOT: sample_profile_suffix_elision_policy +func.func @_QPexternal_sub(%arg0: !fir.ref<i32>) { + return +} diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td index e670e6699e57d..893eee9969f86 100644 --- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td +++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td @@ -2106,7 +2106,8 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [ OptionalAttr<DenseI32ArrayAttr>:$reqd_work_group_size, OptionalAttr<I32Attr>:$intel_reqd_sub_group_size, OptionalAttr<UWTableKindAttr>:$uwtable_kind, - OptionalAttr<BoolAttr>:$use_sample_profile + OptionalAttr<BoolAttr>:$use_sample_profile, + OptionalAttr<StrAttr>:$sample_profile_suffix_elision_policy ); let regions = (region AnyRegion:$body); diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp index 2ab4529ddef53..790fe60f6e998 100644 --- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp @@ -2856,6 +2856,7 @@ static constexpr std::array kExplicitLLVMFuncOpAttributes{ StringLiteral("save-reg-params"), StringLiteral("target-features"), StringLiteral("trap-func-name"), + StringLiteral("sample-profile-suffix-elision-policy"), StringLiteral("tune-cpu"), StringLiteral("uwtable"), StringLiteral("vscale_range"), @@ -3020,6 +3021,12 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func, if (func->hasFnAttribute("use-sample-profile")) funcOp.setUseSampleProfile(true); + if (llvm::Attribute attr = + func->getFnAttribute("sample-profile-suffix-elision-policy"); + attr.isStringAttribute()) + funcOp.setSampleProfileSuffixElisionPolicy( + StringAttr::get(context, attr.getValueAsString())); + if (llvm::Attribute attr = func->getFnAttribute("target-cpu"); attr.isStringAttribute()) funcOp.setTargetCpuAttr(StringAttr::get(context, attr.getValueAsString())); diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp index b87a581a5185e..c514113611103 100644 --- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp @@ -1706,6 +1706,11 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) { if (func.getUseSampleProfile()) llvmFunc->addFnAttr("use-sample-profile"); + if (auto sampleProfileSuffixElisionPolicy = + func.getSampleProfileSuffixElisionPolicy()) + llvmFunc->addFnAttr("sample-profile-suffix-elision-policy", + *sampleProfileSuffixElisionPolicy); + if (auto attr = func.getVscaleRange()) llvmFunc->addFnAttr(llvm::Attribute::getWithVScaleRangeArgs( getLLVMContext(), attr->getMinRange().getInt(), diff --git a/mlir/test/Target/LLVMIR/Import/sample-profile-suffix-elision-policy.ll b/mlir/test/Target/LLVMIR/Import/sample-profile-suffix-elision-policy.ll new file mode 100644 index 0000000000000..8f9a59fea827c --- /dev/null +++ b/mlir/test/Target/LLVMIR/Import/sample-profile-suffix-elision-policy.ll @@ -0,0 +1,15 @@ +; RUN: mlir-translate -import-llvm %s | FileCheck %s + +; CHECK-LABEL: llvm.func @with_elision_policy() +; CHECK-SAME: sample_profile_suffix_elision_policy = "selected" +define void @with_elision_policy() #0 { + ret void +} + +; CHECK-LABEL: llvm.func @without_elision_policy() +; CHECK-NOT: sample_profile_suffix_elision_policy +define void @without_elision_policy() { + ret void +} + +attributes #0 = { "sample-profile-suffix-elision-policy"="selected" } diff --git a/mlir/test/Target/LLVMIR/sample-profile-suffix-elision-policy.mlir b/mlir/test/Target/LLVMIR/sample-profile-suffix-elision-policy.mlir new file mode 100644 index 0000000000000..4882a886d4558 --- /dev/null +++ b/mlir/test/Target/LLVMIR/sample-profile-suffix-elision-policy.mlir @@ -0,0 +1,7 @@ +// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s + +// CHECK: define void @with_elision_policy() #[[ATTRS:.*]] { +// CHECK: attributes #[[ATTRS]] = { "sample-profile-suffix-elision-policy"="selected" } +llvm.func @with_elision_policy() attributes {sample_profile_suffix_elision_policy = "selected"} { + llvm.return +} `````````` </details> https://github.com/llvm/llvm-project/pull/216680 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
