[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
@@ -0,0 +1,86 @@ +//===-- llvm/GEPNoWrapFlags.h - NoWrap flags for GEPs ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file defines the nowrap flags for getelementptr operators. +// +//===--===// + +#ifndef LLVM_IR_GEPNOWRAPFLAGS_H +#define LLVM_IR_GEPNOWRAPFLAGS_H + +namespace llvm { + +class GEPNoWrapFlags { + enum : unsigned{ +InBoundsFlag = (1 << 0), +NUSWFlag = (1 << 1), +NUWFlag = (1 << 2), + }; + + unsigned Flags; + GEPNoWrapFlags(unsigned Flags) : Flags(Flags) { +assert((!isInBounds() || hasNoUnsignedSignedWrap()) && + "inbounds implies nusw"); + } + +public: + GEPNoWrapFlags() : Flags(0) {} + // For historical reasons, interpret plain boolean as InBounds. aeubanks wrote: TODO: remove? https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
https://github.com/aeubanks approved this pull request. thanks, I think abstracting out GEPNoWrapFlags is good https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
@@ -0,0 +1,86 @@ +//===-- llvm/GEPNoWrapFlags.h - NoWrap flags for GEPs ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file defines the nowrap flags for getelementptr operators. +// +//===--===// + +#ifndef LLVM_IR_GEPNOWRAPFLAGS_H +#define LLVM_IR_GEPNOWRAPFLAGS_H + +namespace llvm { + +class GEPNoWrapFlags { + enum : unsigned{ aeubanks wrote: clang-format https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -1028,6 +1029,14 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, Phase != ThinOrFullLTOPhase::ThinLTOPostLink) MPM.addPass(SampleProfileProbePass(TM)); + // Instrument function entry and exit before all inlining. + if (Phase != ThinOrFullLTOPhase::ThinLTOPostLink && + Phase != ThinOrFullLTOPhase::FullLTOPostLink && + Phase != ThinOrFullLTOPhase::None) { aeubanks wrote: we need this pass when `Phase == ThinOrFullLTOPhase::None` right? I'd extract out `isLTOPostLink` to mirror the existing `isLTOPreLink` https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks commented: looks pretty good to me for testing the pre-inliner one, we should add some tests in `llvm/test/Transforms/EntryExitInstrumenter/` that invoke things like `opt -passes='default'`, `opt -passes='thinlto-pre-link'`, `opt -passes='thinlto'` to make sure that the pass did/didn't insert the call given some IR with the appropriate function attribute for testing the post-inliner one, an `llc` x86-64 test that checks that a call to the function was generated in the assembly given some IR with the appropriate function attribute is enough https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -101,6 +101,7 @@ void initializeEarlyMachineLICMPass(PassRegistry&); void initializeEarlyTailDuplicatePass(PassRegistry&); void initializeEdgeBundlesPass(PassRegistry&); void initializeEHContGuardCatchretPass(PassRegistry &); +void initializeEntryExitInstrumenterPass(PassRegistry&); aeubanks wrote: not necessary https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
aeubanks wrote: can you add links to https://reviews.llvm.org/D97608, https://github.com/rust-lang/rust/issues/92109, https://github.com/llvm/llvm-project/issues/52853 https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -135,6 +138,65 @@ static bool runOnFunction(Function , bool PostInlining) { return Changed; } +namespace { +struct EntryExitInstrumenter : public FunctionPass { + static char ID; + EntryExitInstrumenter() : FunctionPass(ID) { +initializeEntryExitInstrumenterPass(*PassRegistry::getPassRegistry()); + } + void getAnalysisUsage(AnalysisUsage ) const override { +AU.addPreserved(); +AU.addPreserved(); + } + bool runOnFunction(Function ) override { return ::runOnFunction(F, false); } +}; +char EntryExitInstrumenter::ID = 0; + +struct PostInlineEntryExitInstrumenter : public FunctionPass { + static char ID; + PostInlineEntryExitInstrumenter() : FunctionPass(ID) { +initializePostInlineEntryExitInstrumenterPass( +*PassRegistry::getPassRegistry()); + } + void getAnalysisUsage(AnalysisUsage ) const override { +AU.addPreserved(); +AU.addPreserved(); + } + bool runOnFunction(Function ) override { return ::runOnFunction(F, true); } +}; +char PostInlineEntryExitInstrumenter::ID = 0; +} + +INITIALIZE_PASS_BEGIN( +EntryExitInstrumenter, "ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() (pre inlining)", +false, false) +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) +INITIALIZE_PASS_END( +EntryExitInstrumenter, "ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() (pre inlining)", +false, false) + +INITIALIZE_PASS_BEGIN( +PostInlineEntryExitInstrumenter, "post-inline-ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() " +"(post inlining)", +false, false) +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) +INITIALIZE_PASS_END( +PostInlineEntryExitInstrumenter, "post-inline-ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() " +"(post inlining)", +false, false) + +FunctionPass *llvm::createEntryExitInstrumenterPass() { aeubanks wrote: we don't need this one since we're adding it in the optimization pipeline which only works with the new pass manager https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -670,9 +670,6 @@ void CodeGenPassBuilder::addIRPasses( !Opt.DisablePartialLibcallInlining) addPass(PartiallyInlineLibCallsPass()); - // Instrument function entry and exit, e.g. with calls to mcount(). - addPass(EntryExitInstrumenterPass(/*PostInlining=*/true)); aeubanks wrote: don't touch this one, this is the WIP port of the codegen pipeline to the new pass manager (and this is actually what we want, that the codegen pipeline does this) https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -1016,6 +1000,11 @@ void EmitAssemblyHelper::RunOptimizationPipeline( if (!IsThinLTOPostLink) { addSanitizers(TargetTriple, CodeGenOpts, LangOpts, PB); addKCFIPass(TargetTriple, LangOpts, PB); + PB.registerPipelineStartEPCallback( aeubanks wrote: we should be consistent and make both entry exit instrumenters part of the default pipelines, not a clang-specific add-on this would probably be somewhere in `buildModuleSimplificationPipeline`, checking `Phase` (and make sure -O0 works by modifying `buildO0DefaultPipeline`) https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Revert "Generate assume loads only with -fstrict-vtable-pointers" (PR #91900)
aeubanks wrote: > -fstrict-vtable-pointers IS experimental, but if you recall, this particular > optimization was added to -fstrict-vtable-pointers because of the effects it > had on compile-time, not because of correctness issues. can you clarify what you mean by "this particular optimization"? you mean adding or not adding assume loads? when I said "regress", I meant runtime performance, not compile times, I should have been clearer https://github.com/llvm/llvm-project/pull/91900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Revert "Generate assume loads only with -fstrict-vtable-pointers" (PR #91900)
aeubanks wrote: adding assumes in general has issues: https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 do you have proof that this change helps binaries and doesn't regress things? I have a feeling this will regress many things. `-fstrict-vtable-pointers` is still somewhat experimental https://github.com/llvm/llvm-project/pull/91900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (#91498)" (PR #91620)
aeubanks wrote: should be fine to revert as much as you want in a single PR, just make sure to mention what you're reverting in the description https://github.com/llvm/llvm-project/pull/91620 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (PR #91498)
aeubanks wrote: Chromium is also seeing similar breakages. @sdkrystian is this breaking valid code? I can't tell from your latest comment. (if it is breaking valid code we should revert) https://github.com/llvm/llvm-project/pull/91498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
aeubanks wrote: btw we're still looking into a performance regression caused by #68882 that still repros with LLVM head, even after the SROA enhancements. this patch looks good, but can we hold off a bit on submitting this? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
@@ -944,43 +943,18 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP, return ConstantExpr::getIntToPtr(C, ResTy); } - // Otherwise form a regular getelementptr. Recompute the indices so that - // we eliminate over-indexing of the notional static type array bounds. - // This makes it easy to determine if the getelementptr is "inbounds". - - // For GEPs of GlobalValues, use the value type, otherwise use an i8 GEP. - if (auto *GV = dyn_cast(Ptr)) -SrcElemTy = GV->getValueType(); - else -SrcElemTy = Type::getInt8Ty(Ptr->getContext()); - - if (!SrcElemTy->isSized()) -return nullptr; - - Type *ElemTy = SrcElemTy; - SmallVector Indices = DL.getGEPIndicesForOffset(ElemTy, Offset); - if (Offset != 0) -return nullptr; - - // Try to add additional zero indices to reach the desired result element - // type. - // TODO: Should we avoid extra zero indices if ResElemTy can't be reached and - // we'll have to insert a bitcast anyway? - while (ElemTy != ResElemTy) { -Type *NextTy = GetElementPtrInst::getTypeAtIndex(ElemTy, (uint64_t)0); -if (!NextTy) - break; - -Indices.push_back(APInt::getZero(isa(ElemTy) ? 32 : BitWidth)); -ElemTy = NextTy; + // Try to infer inbounds for GEPs of globals. + if (!InBounds && Offset.isNonNegative()) { aeubanks wrote: is there a test for the case where `!Offset.isNonNegative()`? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
@@ -944,43 +943,18 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP, return ConstantExpr::getIntToPtr(C, ResTy); } - // Otherwise form a regular getelementptr. Recompute the indices so that - // we eliminate over-indexing of the notional static type array bounds. - // This makes it easy to determine if the getelementptr is "inbounds". - - // For GEPs of GlobalValues, use the value type, otherwise use an i8 GEP. - if (auto *GV = dyn_cast(Ptr)) -SrcElemTy = GV->getValueType(); - else -SrcElemTy = Type::getInt8Ty(Ptr->getContext()); - - if (!SrcElemTy->isSized()) -return nullptr; - - Type *ElemTy = SrcElemTy; - SmallVector Indices = DL.getGEPIndicesForOffset(ElemTy, Offset); - if (Offset != 0) -return nullptr; - - // Try to add additional zero indices to reach the desired result element - // type. - // TODO: Should we avoid extra zero indices if ResElemTy can't be reached and - // we'll have to insert a bitcast anyway? - while (ElemTy != ResElemTy) { -Type *NextTy = GetElementPtrInst::getTypeAtIndex(ElemTy, (uint64_t)0); -if (!NextTy) - break; - -Indices.push_back(APInt::getZero(isa(ElemTy) ? 32 : BitWidth)); -ElemTy = NextTy; + // Try to infer inbounds for GEPs of globals. + if (!InBounds && Offset.isNonNegative()) { +bool CanBeNull, CanBeFreed; +uint64_t DerefBytes = +Ptr->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed); +InBounds = DerefBytes != 0 && !CanBeNull && Offset.sle(DerefBytes); aeubanks wrote: can we remove the other constant folding? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add flag to experiment with cold function attributes (PR #89298)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/89298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add flag to experiment with cold function attributes (PR #89298)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/89298 To be removed and promoted to a proper driver flag if experiments turn out fruitful. For now, this can be experimented with `-mllvm -pgo-cold-func-opt=[optsize|minsize|optnone|default] -mllvm -enable-pgo-force-function-attrs`. Original LLVM patch for this functionality: #69030 >From 1c69510ab5a92998cc443100ecfb551776fc03a0 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 15 Apr 2024 20:40:43 + Subject: [PATCH] [clang] Add flag to experiment with cold function attributes To be removed and promoted to a proper driver flag if experiments turn out fruitful. For now, this can be experimented with `-mllvm -pgo-cold-func-opt=[optsize|minsize|optnone|default] -mllvm -enable-pgo-force-function-attrs`. Original LLVM patch for this functionality: #69030 --- clang/lib/CodeGen/BackendUtil.cpp | 57 --- .../test/CodeGen/pgo-force-function-attrs.ll | 12 2 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 clang/test/CodeGen/pgo-force-function-attrs.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 6cc00b85664f41..22c3f8642ad8eb 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -104,6 +104,21 @@ static cl::opt ClSanitizeOnOptimizerEarlyEP( "sanitizer-early-opt-ep", cl::Optional, cl::desc("Insert sanitizers on OptimizerEarlyEP.")); +// Experiment to mark cold functions as optsize/minsize/optnone. +// TODO: remove once this is exposed as a proper driver flag. +static cl::opt ClPGOColdFuncAttr( +"pgo-cold-func-opt", cl::init(PGOOptions::ColdFuncOpt::Default), cl::Hidden, +cl::desc( +"Function attribute to apply to cold functions as determined by PGO"), +cl::values(clEnumValN(PGOOptions::ColdFuncOpt::Default, "default", + "Default (no attribute)"), + clEnumValN(PGOOptions::ColdFuncOpt::OptSize, "optsize", + "Mark cold functions with optsize."), + clEnumValN(PGOOptions::ColdFuncOpt::MinSize, "minsize", + "Mark cold functions with minsize."), + clEnumValN(PGOOptions::ColdFuncOpt::OptNone, "optnone", + "Mark cold functions with optnone."))); + extern cl::opt ProfileCorrelate; // Re-link builtin bitcodes after optimization @@ -768,42 +783,41 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. auto CSAction = CodeGenOpts.hasProfileCSIRUse() ? PGOOptions::CSIRUse : PGOOptions::NoCSAction; -PGOOpt = PGOOptions( -CodeGenOpts.ProfileInstrumentUsePath, "", -CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +PGOOpt = PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "", +CodeGenOpts.ProfileRemappingFile, +CodeGenOpts.MemoryProfileUsePath, VFS, +PGOOptions::IRUse, CSAction, ClPGOColdFuncAttr, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling -PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, -
[clang] [clang] Add flag to experiment with cold function attributes (PR #88793)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/88793 To be removed and promoted to a proper driver flag if experiments turn out fruitful. Original LLVM patch for this functionality: #69030 >From 52cd9974be908bf693832012e56e945e9e34f389 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 15 Apr 2024 20:40:43 + Subject: [PATCH] [clang] Add flag to experiment with cold function attributes To be removed and promoted to a proper driver flag if experiments turn out fruitful. Original LLVM patch for this functionality: #69030 --- clang/lib/CodeGen/BackendUtil.cpp | 57 +++ 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 6cc00b85664f41..22c3f8642ad8eb 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -104,6 +104,21 @@ static cl::opt ClSanitizeOnOptimizerEarlyEP( "sanitizer-early-opt-ep", cl::Optional, cl::desc("Insert sanitizers on OptimizerEarlyEP.")); +// Experiment to mark cold functions as optsize/minsize/optnone. +// TODO: remove once this is exposed as a proper driver flag. +static cl::opt ClPGOColdFuncAttr( +"pgo-cold-func-opt", cl::init(PGOOptions::ColdFuncOpt::Default), cl::Hidden, +cl::desc( +"Function attribute to apply to cold functions as determined by PGO"), +cl::values(clEnumValN(PGOOptions::ColdFuncOpt::Default, "default", + "Default (no attribute)"), + clEnumValN(PGOOptions::ColdFuncOpt::OptSize, "optsize", + "Mark cold functions with optsize."), + clEnumValN(PGOOptions::ColdFuncOpt::MinSize, "minsize", + "Mark cold functions with minsize."), + clEnumValN(PGOOptions::ColdFuncOpt::OptNone, "optnone", + "Mark cold functions with optnone."))); + extern cl::opt ProfileCorrelate; // Re-link builtin bitcodes after optimization @@ -768,42 +783,41 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. auto CSAction = CodeGenOpts.hasProfileCSIRUse() ? PGOOptions::CSIRUse : PGOOptions::NoCSAction; -PGOOpt = PGOOptions( -CodeGenOpts.ProfileInstrumentUsePath, "", -CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +PGOOpt = PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "", +CodeGenOpts.ProfileRemappingFile, +CodeGenOpts.MemoryProfileUsePath, VFS, +PGOOptions::IRUse, CSAction, ClPGOColdFuncAttr, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling -PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling, true); +PGOOpt = +PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, + PGOOptions::NoAction, PGOOptions::NoCSAction, + ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) //
[clang] [flang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #88661)
https://github.com/aeubanks approved this pull request. lg, but update the commit message `Pull Request: https://github.com/llvm/llvm-project/pull/87866`, that's obsolete https://github.com/llvm/llvm-project/pull/88661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: yeah that patch makes those test pass with this PR, lgtm (you could also test locally by touching the files I mentioned above, e.g. even just `touch lib/clang/19/lib/linux/libclang_rt.builtins-aarch64-android.a` repro'd the test failure on my machine) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] be10070 - Revert "[Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin"
Author: Arthur Eubanks Date: 2024-04-10T23:41:51Z New Revision: be10070f91b86a6f126d2451852242bfcb2cd366 URL: https://github.com/llvm/llvm-project/commit/be10070f91b86a6f126d2451852242bfcb2cd366 DIFF: https://github.com/llvm/llvm-project/commit/be10070f91b86a6f126d2451852242bfcb2cd366.diff LOG: Revert "[Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin" This reverts commit ccdebbae4d77d3efc236af92c22941de5d437e01. Causes test failures in the presence of Android runtime libraries in resource-dir. See comments on https://github.com/llvm/llvm-project/pull/87866. Added: Modified: clang/lib/Driver/ToolChain.cpp clang/test/Driver/arm-compiler-rt.c clang/test/Driver/cl-link.c clang/test/Driver/compiler-rt-unwind.c clang/test/Driver/coverage-ld.c clang/test/Driver/instrprof-ld.c clang/test/Driver/linux-ld.c clang/test/Driver/mingw-sanitizers.c clang/test/Driver/msp430-toolchain.c clang/test/Driver/print-libgcc-file-name-clangrt.c clang/test/Driver/print-runtime-dir.c clang/test/Driver/riscv32-toolchain-extra.c clang/test/Driver/riscv32-toolchain.c clang/test/Driver/riscv64-toolchain-extra.c clang/test/Driver/riscv64-toolchain.c clang/test/Driver/sanitizer-ld.c clang/test/Driver/wasm-toolchain.c clang/test/Driver/wasm-toolchain.cpp clang/test/Driver/windows-cross.c clang/test/Driver/zos-ld.c flang/test/Driver/msvc-dependent-lib-flags.f90 Removed: diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 237092ed07e5dc..03450fc0f57b93 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -796,13 +796,7 @@ ToolChain::getTargetSubDirPath(StringRef BaseDir) const { std::optional ToolChain::getRuntimePath() const { SmallString<128> P(D.ResourceDir); llvm::sys::path::append(P, "lib"); - if (auto Ret = getTargetSubDirPath(P)) -return Ret; - // Darwin does not use per-target runtime directory. - if (Triple.isOSDarwin()) -return {}; - llvm::sys::path::append(P, Triple.str()); - return std::string(P); + return getTargetSubDirPath(P); } std::optional ToolChain::getStdlibPath() const { diff --git a/clang/test/Driver/arm-compiler-rt.c b/clang/test/Driver/arm-compiler-rt.c index cb6c29f48a7814..5e9e528400d08e 100644 --- a/clang/test/Driver/arm-compiler-rt.c +++ b/clang/test/Driver/arm-compiler-rt.c @@ -10,47 +10,47 @@ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABI -// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a" // RUN: %clang -target arm-linux-gnueabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABI-ABI -// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins-armhf.a" // RUN: %clang -target arm-linux-gnueabihf \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF -// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins-armhf.a" // RUN: %clang -target arm-linux-gnueabihf \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -mfloat-abi=soft -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF-ABI -// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a" // RUN: %clang -target arm-windows-itanium \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-WINDOWS -// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins.lib" +// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins-arm.lib" // RUN: %clang -target arm-linux-androideabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-ANDROID -// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins-arm-android.a" // RUN: not %clang --target=arm-linux-androideabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN:
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: it seems that the test depends on if certain android runtime libraries are present or not in the resource dir (without per-target runtime directory since that's still an issue on Android). perhaps the tests need `-resource-dir` to make them more hermetic? anyway, will revert, feel free to reland once the tests are fixed. this is what's in my resource dir if it's helpful for reproducing ``` $ ls lib/clang/19/lib/linux/ aarch64 libclang_rt.asan_cxx-aarch64-android.a libclang_rt.builtins-aarch64-android.a libclang_rt.tsan_cxx-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.so libclang_rt.asan-aarch64-android.a libclang_rt.asan-preinit-aarch64-android.a libclang_rt.profile-aarch64-android.a libclang_rt.ubsan_minimal-aarch64-android.a libclang_rt.ubsan_standalone_cxx-aarch64-android.a libclang_rt.asan-aarch64-android.so libclang_rt.asan_static-aarch64-android.a libclang_rt.tsan-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.a ``` https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: this seemes to be causing some test failures for us: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8751043232043110529/+/u/package_clang/stdout?format=raw ``` TEST 'Clang :: Driver/linux-ld.c' FAILED RUN: at line 92: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/linux-ld.c:102:25: error: CHECK-LD-RT-ANDROID: expected string not found in input // CHECK-LD-RT-ANDROID: libclang_rt.builtins.a" ^ :6:331: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" ^ :6:866: note: possible intended match here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" TEST 'Clang :: Driver/sanitizer-ld.c' FAILED RUN: at line 177: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/sanitizer-ld.c:187:24: error: CHECK-ASAN-ANDROID: expected string not found in input // CHECK-ASAN-ANDROID: libclang_rt.asan.so" ^ :6:320: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-pie" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan-arm-android.so" "--whole-archive" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan_static-arm-android.a" "--no-whole-archive" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/sanitizer-ld-d1ddb3.o"
[clang] Reland "[Win32][ELF] Make CodeView a DebugInfoFormat only for COFF format" (PR #87987)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/87987 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
aeubanks wrote: > I'd suggest adding bitcode upgrade if it isn't too hard (I don't think it > should be?) done https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/87906 >From 7a9df42b4c4f4f1b02dc3158d24800f3d4b68d8f Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Sun, 7 Apr 2024 05:29:36 + Subject: [PATCH 1/2] [clang][llvm] Remove "implicit-section-name" attribute D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. --- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/test/CodeGen/clang-sections-attribute.c | 3 --- clang/test/CodeGenCXX/clang-sections.cpp | 18 ++--- llvm/lib/CodeGen/TargetInstrInfo.cpp | 3 +-- .../CodeGen/TargetLoweringObjectFileImpl.cpp | 11 +--- llvm/lib/Target/TargetLoweringObjectFile.cpp | 5 .../CodeGen/AArch64/clang-section-macho.ll| 22 --- llvm/test/CodeGen/ARM/clang-section.ll| 24 - .../Generic/machine-function-splitter.ll | 27 +++ .../basic-block-sections-pragma-sections.ll | 4 +-- 10 files changed, 15 insertions(+), 104 deletions(-) delete mode 100644 llvm/test/CodeGen/AArch64/clang-section-macho.ll diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 00b3bfcaa0bc25..f4dbfe7a21f83c 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2627,7 +2627,7 @@ void CodeGenModule::setNonAliasAttributes(GlobalDecl GD, addUsedGlobal(F); if (auto *SA = D->getAttr()) if (!D->getAttr()) - F->addFnAttr("implicit-section-name", SA->getName()); + F->setSection(SA->getName()); llvm::AttrBuilder Attrs(F->getContext()); if (GetCPUAndFeaturesAttributes(GD, Attrs)) { diff --git a/clang/test/CodeGen/clang-sections-attribute.c b/clang/test/CodeGen/clang-sections-attribute.c index 70ed24ed07a280..768bdd4d87649e 100644 --- a/clang/test/CodeGen/clang-sections-attribute.c +++ b/clang/test/CodeGen/clang-sections-attribute.c @@ -69,8 +69,5 @@ static int int_zvar; // CHECK: define internal void @int_fun() #0 section ".int_fun_attr" // CHECK: define internal void @int_fun2() #0 section ".int_fun2_attr" // -// Function attributes should not include implicit-section-name. -// CHECK-NOT: attributes #0 = {{.*}}implicit-section-name -// // No other attribute group should be present in the file. // CHECK-NOT: attributes #1 diff --git a/clang/test/CodeGenCXX/clang-sections.cpp b/clang/test/CodeGenCXX/clang-sections.cpp index a444f2d0cae59c..aa159e552b1b3c 100644 --- a/clang/test/CodeGenCXX/clang-sections.cpp +++ b/clang/test/CodeGenCXX/clang-sections.cpp @@ -81,24 +81,22 @@ int hoo(void) { //CHECK: @p ={{.*}} constant i32 7, align 4 //CHECK: @_ZL5fptrs = internal constant [2 x ptr] [ptr @foo, ptr @goo], align {{4|8}} #3 -//CHECK: define{{.*}} i32 @foo() #5 { -//CHECK: define{{.*}} i32 @goo() #6 { -//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #7 -//CHECK: define{{.*}} i32 @hoo() #8 { +//ELF: define{{.*}} i32 @foo(){{.*}} section "my_text.1" { +//ELF: define{{.*}} i32 @goo(){{.*}} section "my_text.2" { +//MACHO: define{{.*}} i32 @foo(){{.*}} section "__TEXT,__mytext1" { +//MACHO: define{{.*}} i32 @goo(){{.*}} section "__TEXT,__mytext2" { + +// ensure zoo/hoo don't have a section +//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #6{{$}} +//CHECK: define{{.*}} i32 @hoo() #5 { //ELF: attributes #0 = { "bss-section"="my_bss.1" "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #1 = { "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #2 = { "bss-section"="my_bss.2" "rodata-section"="my_rodata.1" } //ELF: attributes #3 = { "bss-section"="my_bss.2" "data-section"="my_data.2" "relro-section"="my_relro.2" "rodata-section"="my_rodata.2" } //ELF: attributes #4 = { "relro-section"="my_relro.2" } -//ELF: attributes #5 = { {{.*"implicit-section-name"="my_text.1".*}} } -//ELF: attributes #6 = { {{.*"implicit-section-name"="my_text.2".*}} } //MACHO: attributes #0 = { "bss-section"="__BSS,__mybss1" "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #1 = { "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #2 = { "bss-section"="__BSS,__mybss2" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #3 = { "bss-section"="__BSS,__mybss2" "data-section"="__DATA,__mydata2" "relro-section"="__RELRO,__myrelro2" "rodata-section"="__RODATA,__myrodata2" } //MACHO: attributes #4 = { "relro-section"="__RELRO,__myrelro2" } -//MACHO: attributes #5 = { {{.*"implicit-section-name"="__TEXT,__mytext1".*}} } -//MACHO: attributes #6 = {
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
aeubanks wrote: this probably needs bitcode upgrade? https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/87906 D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. >From 7a9df42b4c4f4f1b02dc3158d24800f3d4b68d8f Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Sun, 7 Apr 2024 05:29:36 + Subject: [PATCH] [clang][llvm] Remove "implicit-section-name" attribute D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. --- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/test/CodeGen/clang-sections-attribute.c | 3 --- clang/test/CodeGenCXX/clang-sections.cpp | 18 ++--- llvm/lib/CodeGen/TargetInstrInfo.cpp | 3 +-- .../CodeGen/TargetLoweringObjectFileImpl.cpp | 11 +--- llvm/lib/Target/TargetLoweringObjectFile.cpp | 5 .../CodeGen/AArch64/clang-section-macho.ll| 22 --- llvm/test/CodeGen/ARM/clang-section.ll| 24 - .../Generic/machine-function-splitter.ll | 27 +++ .../basic-block-sections-pragma-sections.ll | 4 +-- 10 files changed, 15 insertions(+), 104 deletions(-) delete mode 100644 llvm/test/CodeGen/AArch64/clang-section-macho.ll diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 00b3bfcaa0bc25..f4dbfe7a21f83c 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2627,7 +2627,7 @@ void CodeGenModule::setNonAliasAttributes(GlobalDecl GD, addUsedGlobal(F); if (auto *SA = D->getAttr()) if (!D->getAttr()) - F->addFnAttr("implicit-section-name", SA->getName()); + F->setSection(SA->getName()); llvm::AttrBuilder Attrs(F->getContext()); if (GetCPUAndFeaturesAttributes(GD, Attrs)) { diff --git a/clang/test/CodeGen/clang-sections-attribute.c b/clang/test/CodeGen/clang-sections-attribute.c index 70ed24ed07a280..768bdd4d87649e 100644 --- a/clang/test/CodeGen/clang-sections-attribute.c +++ b/clang/test/CodeGen/clang-sections-attribute.c @@ -69,8 +69,5 @@ static int int_zvar; // CHECK: define internal void @int_fun() #0 section ".int_fun_attr" // CHECK: define internal void @int_fun2() #0 section ".int_fun2_attr" // -// Function attributes should not include implicit-section-name. -// CHECK-NOT: attributes #0 = {{.*}}implicit-section-name -// // No other attribute group should be present in the file. // CHECK-NOT: attributes #1 diff --git a/clang/test/CodeGenCXX/clang-sections.cpp b/clang/test/CodeGenCXX/clang-sections.cpp index a444f2d0cae59c..aa159e552b1b3c 100644 --- a/clang/test/CodeGenCXX/clang-sections.cpp +++ b/clang/test/CodeGenCXX/clang-sections.cpp @@ -81,24 +81,22 @@ int hoo(void) { //CHECK: @p ={{.*}} constant i32 7, align 4 //CHECK: @_ZL5fptrs = internal constant [2 x ptr] [ptr @foo, ptr @goo], align {{4|8}} #3 -//CHECK: define{{.*}} i32 @foo() #5 { -//CHECK: define{{.*}} i32 @goo() #6 { -//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #7 -//CHECK: define{{.*}} i32 @hoo() #8 { +//ELF: define{{.*}} i32 @foo(){{.*}} section "my_text.1" { +//ELF: define{{.*}} i32 @goo(){{.*}} section "my_text.2" { +//MACHO: define{{.*}} i32 @foo(){{.*}} section "__TEXT,__mytext1" { +//MACHO: define{{.*}} i32 @goo(){{.*}} section "__TEXT,__mytext2" { + +// ensure zoo/hoo don't have a section +//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #6{{$}} +//CHECK: define{{.*}} i32 @hoo() #5 { //ELF: attributes #0 = { "bss-section"="my_bss.1" "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #1 = { "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #2 = { "bss-section"="my_bss.2" "rodata-section"="my_rodata.1" } //ELF: attributes #3 = { "bss-section"="my_bss.2" "data-section"="my_data.2" "relro-section"="my_relro.2" "rodata-section"="my_rodata.2" } //ELF: attributes #4 = { "relro-section"="my_relro.2" } -//ELF: attributes #5 = { {{.*"implicit-section-name"="my_text.1".*}} } -//ELF: attributes #6 = { {{.*"implicit-section-name"="my_text.2".*}} } //MACHO: attributes #0 = { "bss-section"="__BSS,__mybss1" "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #1 = { "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #2 = { "bss-section"="__BSS,__mybss2" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #3 = {
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: ah I see. I feel like this should be a more principled approach that other sanitizers also share, as you've mentioned as an alternative. do people not care about other sanitizers in production? (I'm going to be OOO for a week, so someone else will need to review) https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: > We already have similar stuff: > > ``` > if (PGOOpt && Phase != ThinOrFullLTOPhase::ThinLTOPostLink && > !PGOOpt->MemoryProfile.empty()) > MPM.addPass(MemProfUsePass(PGOOpt->MemoryProfile, PGOOpt->FS)); > ``` checking for ThinLTO pre/post link is a correctness thing though I think I'm still confused on exactly what the use case is and why we can't just ask the user to not specify hwasan in the PGO instrumented build. Just for user convenience? Or does clang change the emitted IR when hwasan is enabled? And that's what will lead to mismatched profiles? https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: > > why can't hwasan and PGO instrumentation coexist? > > They can, but binary is like 5x times slower, on top of 10x slowdown of PGO > instrumentation. (don't quote me on these numbers, they are from large but > single benchmark, still it's very slow) > If it's usable as a configuration, I don't see why we should prevent this. It still may be useful to some people. Seems like this checking should be done at a build system level if you don't want some codebase to compile with this configuration. > > and this seems like it should be an error at the clang driver level, > > instead of silently turning off one of the requested features > > 1. We need -fsanitizer=hwaddress, for attributes and profile matching, and > some special handling done in earlier passes. Do you mean that if you want a hwasan/PGO optimized build, you want the corresponding PGO instrumented build to also use hwasan? Doesn't PGO instrumentation/use happen before the sanitizer passes run? > 2. We don't wan't users care about profile instrumentation/use difference. https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: why can't hwasan and PGO instrumentation coexist? and this seems like it should be an error at the clang driver level, instead of silently turning off one of the requested features https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
@@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept aeubanks wrote: done https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/86039 >From bba8e4003c4ccc36497e62ad1696197e6987525c Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 20 Mar 2024 23:36:35 + Subject: [PATCH 1/2] [clang][SPIRV] Ignore -mcmodel The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- clang/test/Driver/unsupported-option-gpu.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 055884d275ce1b..035bfa35299756 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept // whatever works for the host. Ok = true; diff --git a/clang/test/Driver/unsupported-option-gpu.c b/clang/test/Driver/unsupported-option-gpu.c index f23cb71ebfb08e..5618b2cba72e16 100644 --- a/clang/test/Driver/unsupported-option-gpu.c +++ b/clang/test/Driver/unsupported-option-gpu.c @@ -2,4 +2,5 @@ // DEFINE: %{check} = %clang -### --target=x86_64-linux-gnu -c -mcmodel=medium // RUN: %{check} -x cuda %s --cuda-path=%S/Inputs/CUDA/usr/local/cuda --offload-arch=sm_60 --no-cuda-version-check -fbasic-block-sections=all +// RUN: %{check} -x hip %s --offload=spirv64 -nogpulib -nogpuinc // RUN: %{check} -x hip %s --rocm-path=%S/Inputs/rocm -nogpulib -nogpuinc >From 650c120c5ad8360124b4d45a90974f4d60622455 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Thu, 21 Mar 2024 21:44:55 + Subject: [PATCH 2/2] update comment --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 035bfa35299756..57ab8b6e91826c 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5805,7 +5805,7 @@ void Clang::ConstructJob(Compilation , const JobAction , Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); } else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { - // NVPTX/AMDGPU does not care about the code model and will accept + // NVPTX/AMDGPU/SPIRV does not care about the code model and will accept // whatever works for the host. Ok = true; } else if (Triple.isSPARC64()) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Ignore -mcmodel (PR #86039)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/86039 The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. >From bba8e4003c4ccc36497e62ad1696197e6987525c Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 20 Mar 2024 23:36:35 + Subject: [PATCH] [clang][SPIRV] Ignore -mcmodel The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- clang/test/Driver/unsupported-option-gpu.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 055884d275ce1b..035bfa35299756 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept // whatever works for the host. Ok = true; diff --git a/clang/test/Driver/unsupported-option-gpu.c b/clang/test/Driver/unsupported-option-gpu.c index f23cb71ebfb08e..5618b2cba72e16 100644 --- a/clang/test/Driver/unsupported-option-gpu.c +++ b/clang/test/Driver/unsupported-option-gpu.c @@ -2,4 +2,5 @@ // DEFINE: %{check} = %clang -### --target=x86_64-linux-gnu -c -mcmodel=medium // RUN: %{check} -x cuda %s --cuda-path=%S/Inputs/CUDA/usr/local/cuda --offload-arch=sm_60 --no-cuda-version-check -fbasic-block-sections=all +// RUN: %{check} -x hip %s --offload=spirv64 -nogpulib -nogpuinc // RUN: %{check} -x hip %s --rocm-path=%S/Inputs/rocm -nogpulib -nogpuinc ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: > > > yes, but I'd like to that after we collect feedback from first users > > > > > > They are introduced by earlier transformations > > Note: I'd like to have special intrinsic for this optimization. When we > > have it, we likely don't need this SimplifyCFG. > > lgtm with a comment added on why we're adding the extra SimplifyCFG oh I missed that you'd added one. but maybe a TODO with your comment about an intrinsic? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: > > yes, but I'd like to that after we collect feedback from first users > > They are introduced by earlier transformations > > Note: I'd like to have special intrinsic for this optimization. When we have > it, we likely don't need this SimplifyCFG. lgtm with a comment added on why we're adding the extra SimplifyCFG https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: why isn't an earlier SimplifyCFG run in the function simplification pipeline handling these? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: profile matching happens right before the inliner/function simplification pipeline (after the initial `EarlyFPM` and `GlobalCleanupPM` cleanup) https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: I think this can go at the end of the function simplification pipeline (and there's already a SimplifyCFG run right after) with `registerScalarOptimizerLateEPCallback`. The function should already have been mostly simplified at that point. https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: unnecessary `&` https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
aeubanks wrote: the only use of this is GlobalSplit, and it cares about ranges relative to the GlobalVariable, so if we're not planning on using this in more cases then I'd say relative to the source pointer makes sense. not sure if inrange would ever be useful for more than GlobalSplit, like being able to tighten the possible values of a load from a const array if we know the load is in some small range of the array, which doesn't seem super useful https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: is there a long term plan to add a driver flag for this? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Upstream Sema logic to apply API Notes to decls (PR #78445)
aeubanks wrote: > Oh, thanks @nikic for that data point. Let me try to avoid the overhead, I'll > put up a patch tomorrow morning. Did this ever happen? https://github.com/llvm/llvm-project/pull/78445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: Sorry, I thought I had waited long enough and that the previous comments were addressed. Will address your comments in a follow-up. > This is good to go only because it's off by default. Otherwise it's not. > Sample PGO profile has inline context, so in the profile, we may have foo as > cold and bar->foo as hot, but if later inliner rejects bar->foo inlining, foo > can be hot. So marking foo as cold pre-inline can still be inaccurate (and > not conservative). Is this the main objection? I didn't understand this sentence the first time reading it. The Sample PGO inliner runs before this pass so it shouldn't be affected, as mentioned before. By "later inliner" do you mean the normal CGSCC inliner? Is it possible to have a call site be hot but the callee be cold? I don't currently have a plan to make this pass do anything by default, but I would like to resolve objections anyway. Using the `optsize` setting for this pass seems like it could be the default perhaps further in the future. https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
aeubanks wrote: for my information, which version of Visual Studio are you using? https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove old Linux kernel workaround for ensuring stack space (PR #81533)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
aeubanks wrote: seeing if Windows CI catches anything https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/81534 Added in 26670dcba1609574cba5942aff78ff97b567c5f3. >From c659a573a066809473ebb36421e612dcdcda5aef Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 12 Feb 2024 21:01:39 + Subject: [PATCH] [clang] Remove #undef alloca Added in 26670dcba1609574cba5942aff78ff97b567c5f3. --- clang/include/clang/Basic/Builtins.h | 4 1 file changed, 4 deletions(-) diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h index f955d21169556a..6700d1903a0088 100644 --- a/clang/include/clang/Basic/Builtins.h +++ b/clang/include/clang/Basic/Builtins.h @@ -20,10 +20,6 @@ #include "llvm/ADT/StringRef.h" #include -// VC++ defines 'alloca' as an object-like macro, which interferes with our -// builtins. -#undef alloca - namespace clang { class TargetInfo; class IdentifierTable; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove old Linux kernel workaround for ensuring stack space (PR #81533)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/81533 PR #71709 broke the Linux PIE build with `undefined symbol: alloca` errors. With the newly included `clang/Basic/Builtins.h` in that PR, it surfaces an issue with a combination of two previous patches. 26670dcba1609574cba5942aff78ff97b567c5f3 added `#undef alloca` so clang builtins handling of alloca would work under MSVC (unsure if this is still necessary). 194b6a3b1b1a99cc3c12c466a04320f271ebd8aa added code that calls `alloca` to workaround a Linux kernel < 4.1 bug. Given that Linux 4.1 was EOL in 2018, it should be ok to remove this workaround. >From 3dd69256a0a3f9cfabac54cabad5b3dcc2410c36 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 12 Feb 2024 20:51:37 + Subject: [PATCH] [clang] Remove old Linux kernel workaround for ensuring stack space PR #71709 broke the Linux PIE build with `undefined symbol: alloca` errors. With the newly included `clang/Basic/Builtins.h` in that PR, it surfaces an issue with a combination of two previous patches. 26670dcba1609574cba5942aff78ff97b567c5f3 added `#undef alloca` so clang builtins handling of alloca would work under MSVC (unsure if this is still necessary). 194b6a3b1b1a99cc3c12c466a04320f271ebd8aa added code that calls `alloca` to workaround a Linux kernel < 4.1 bug. Given that Linux 4.1 was EOL in 2018, it should be ok to remove this workaround. --- clang/tools/driver/cc1_main.cpp | 62 - 1 file changed, 62 deletions(-) diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp index e9d2c6aad371db..b5c6be3c557bb3 100644 --- a/clang/tools/driver/cc1_main.cpp +++ b/clang/tools/driver/cc1_main.cpp @@ -78,64 +78,6 @@ static void LLVMErrorHandler(void *UserData, const char *Message, } #ifdef CLANG_HAVE_RLIMITS -#if defined(__linux__) && defined(__PIE__) -static size_t getCurrentStackAllocation() { - // If we can't compute the current stack usage, allow for 512K of command - // line arguments and environment. - size_t Usage = 512 * 1024; - if (FILE *StatFile = fopen("/proc/self/stat", "r")) { -// We assume that the stack extends from its current address to the end of -// the environment space. In reality, there is another string literal (the -// program name) after the environment, but this is close enough (we only -// need to be within 100K or so). -unsigned long StackPtr, EnvEnd; -// Disable silly GCC -Wformat warning that complains about length -// modifiers on ignored format specifiers. We want to retain these -// for documentation purposes even though they have no effect. -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat" -#endif -if (fscanf(StatFile, - "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*lu %*lu %*lu %*lu %*lu " - "%*lu %*ld %*ld %*ld %*ld %*ld %*ld %*llu %*lu %*ld %*lu %*lu " - "%*lu %*lu %lu %*lu %*lu %*lu %*lu %*lu %*llu %*lu %*lu %*d %*d " - "%*u %*u %*llu %*lu %*ld %*lu %*lu %*lu %*lu %*lu %*lu %lu %*d", - , ) == 2) { -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic pop -#endif - Usage = StackPtr < EnvEnd ? EnvEnd - StackPtr : StackPtr - EnvEnd; -} -fclose(StatFile); - } - return Usage; -} - -#include - -LLVM_ATTRIBUTE_NOINLINE -static void ensureStackAddressSpace() { - // Linux kernels prior to 4.1 will sometimes locate the heap of a PIE binary - // relatively close to the stack (they are only guaranteed to be 128MiB - // apart). This results in crashes if we happen to heap-allocate more than - // 128MiB before we reach our stack high-water mark. - // - // To avoid these crashes, ensure that we have sufficient virtual memory - // pages allocated before we start running. - size_t Curr = getCurrentStackAllocation(); - const int kTargetStack = DesiredStackSize - 256 * 1024; - if (Curr < kTargetStack) { -volatile char *volatile Alloc = -static_cast(alloca(kTargetStack - Curr)); -Alloc[0] = 0; -Alloc[kTargetStack - Curr - 1] = 0; - } -} -#else -static void ensureStackAddressSpace() {} -#endif - /// Attempt to ensure that we have at least 8MiB of usable stack space. static void ensureSufficientStack() { struct rlimit rlim; @@ -159,10 +101,6 @@ static void ensureSufficientStack() { rlim.rlim_cur != DesiredStackSize) return; } - - // We should now have a stack of size at least DesiredStackSize. Ensure - // that we can actually use that much, if necessary. - ensureStackAddressSpace(); } #else static void ensureSufficientStack() {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor `IdentifierInfo::ObjcOrBuiltinID` (PR #71709)
aeubanks wrote: https://github.com/llvm/llvm-project/issues/4885 for why `#undef alloca` was added https://github.com/llvm/llvm-project/pull/71709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor `IdentifierInfo::ObjcOrBuiltinID` (PR #71709)
aeubanks wrote: I'll send out a PR to remove that code, and potentially also remove the `#undef alloca` separately https://github.com/llvm/llvm-project/pull/71709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor `IdentifierInfo::ObjcOrBuiltinID` (PR #71709)
aeubanks wrote: this seems to break `-fPIE` builds of clang on Linux with the following: ``` ld.lld: error: undefined symbol: alloca >>> referenced by cc1_main.cpp >>> >>> tools/clang/tools/driver/CMakeFiles/clang.dir/cc1_main.cpp.o:(ensureStackAddressSpace()) ``` the call to `alloca` [here](https://github.com/llvm/llvm-project/blob/2fcfc9754a16805b81e541dc8222a8b5cf17a121/clang/tools/driver/cc1_main.cpp#L130) should be a call to `__builtin_alloca` ``` $ cat /usr/include/alloca.h ... /* Remove any previous definition. */ #undef alloca /* Allocate a block that will be freed when the calling function exits. */ extern void *alloca (size_t __size) __THROW; #ifdef __GNUC__ # define alloca(size) __builtin_alloca (size) #endif /* GCC. */ ... ``` But the newly included `clang/Basic/Builtins.h` has [`#undef alloca`](https://github.com/llvm/llvm-project/blob/2fcfc9754a16805b81e541dc8222a8b5cf17a121/clang/include/clang/Basic/Builtins.h#L25) which messes with that. I think we can probably just remove the code in `cc1_main.cpp` since it's a workaround for < Linux 4.1, which has been EOL since 2018. https://github.com/llvm/llvm-project/pull/71709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: > > > Good example. This pass should be run post-inline. @aeubanks, any reason > > > we want to run it early in the pipeline? > > > > > > We want the main function simplification pipeline to see these function > > attributes because some optimizations trigger or don't trigger depending on > > the presence of the attributes. Modifying function attributes is typically > > done in CGSCC/module passes since doing so can affect what callers of those > > functions see (in effect changing other functions), which shouldn't happen > > in function passes. I suppose it's possible to add this as a CGSCC pass > > that runs after inlining and before the function simplification pipeline, > > but this is more of a one time thing and CGSCC passes can revisit > > functions. So this pass makes the most sense as a module pass, but we can't > > insert a module pass between inlining and the function simplification > > pipeline. > > Can/does the inliner ignore these size attributes when it has call-site > > profile information? > > Looking at the current change, this new pass is actually after the sample > loader (including sample loader inlining) pass, so wenlei@'s concern should > be addressed. Oh I forgot that the sample profile has its own inliner. Yes this pass runs after we load profiling information since it uses profiling information, whether it's sample or instrumented. Is this patch good to go? https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: > Good example. This pass should be run post-inline. @aeubanks, any reason we > want to run it early in the pipeline? We want the main function simplification pipeline to see these function attributes because some optimizations trigger or don't trigger depending on the presence of the attributes. Modifying function attributes is typically done in CGSCC/module passes since doing so can affect what callers of those functions see (in effect changing other functions), which shouldn't happen in function passes. I suppose it's possible to add this as a CGSCC pass that runs after inlining and before the function simplification pipeline, but this is more of a one time thing and CGSCC passes can revisit functions. So this pass makes the most sense as a module pass, but we can't insert a module pass between inlining and the function simplification pipeline. Can/does the inliner ignore these size attributes when it has call-site profile information? https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: I don't understand, if you're saying the profile is accurate, then those functions are actually cold, so we should be able to mark them as optsize? https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/69030 >From e52a811c3b643548837b4e630e8293a0b6857ad4 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 13 Oct 2023 14:40:28 -0700 Subject: [PATCH 1/6] [PGO] Add ability to mark cold functions as optsize/minsize/optnone The performance of cold functions shouldn't matter too much, so if we care about binary sizes, add an option to mark cold functions as optsize/minsize for binary size, or optnone for compile times [1]. Clang patch will be in a future patch Initial version: https://reviews.llvm.org/D149800 [1] https://discourse.llvm.org/t/rfc-new-feature-proposal-de-optimizing-cold-functions-using-pgo-info/56388 --- clang/lib/CodeGen/BackendUtil.cpp | 18 ++-- llvm/include/llvm/Support/PGOOptions.h| 3 + .../Instrumentation/MarkColdFunctions.h | 28 ++ llvm/lib/LTO/LTOBackend.cpp | 12 ++- llvm/lib/Passes/PassBuilder.cpp | 1 + llvm/lib/Passes/PassBuilderPipelines.cpp | 12 +++ llvm/lib/Passes/PassRegistry.def | 1 + llvm/lib/Support/PGOOptions.cpp | 7 +- .../Transforms/Instrumentation/CMakeLists.txt | 1 + .../Instrumentation/MarkColdFunctions.cpp | 65 + .../Transforms/MarkColdFunctions/basic.ll | 97 +++ llvm/tools/opt/NewPMDriver.cpp| 24 - .../lib/Transforms/Instrumentation/BUILD.gn | 1 + 13 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h create mode 100644 llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp create mode 100644 llvm/test/Transforms/MarkColdFunctions/basic.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index a6142d99f3b68..7e9d3b8ea55a1 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -746,7 +746,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. @@ -755,28 +756,32 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PGOOpt = PGOOptions( CodeGenOpts.ProfileInstrumentUsePath, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling); +PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, -CodeGenOpts.PseudoProbeForProfiling); +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) // -fdebug-info-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, true); +PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, true); // Check to see if we want to generate a CS profile. if (CodeGenOpts.hasProfileCSIRInstr()) { @@ -799,7 +804,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction,
[llvm] [clang] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -0,0 +1,73 @@ +//===--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h" +#include "llvm/Analysis/BlockFrequencyInfo.h" +#include "llvm/Analysis/ProfileSummaryInfo.h" +#include "llvm/IR/PassManager.h" +#include "llvm/Support/ErrorHandling.h" + +using namespace llvm; + +static bool shouldRunOnFunction(Function , ProfileSummaryInfo , +FunctionAnalysisManager ) { + if (F.hasFnAttribute(Attribute::Cold)) +return true; + if (!PSI.hasProfileSummary()) +return false; + BlockFrequencyInfo = FAM.getResult(F); + return PSI.isFunctionColdInCallGraph(, BFI); +} + +PreservedAnalyses PGOForceFunctionAttrsPass::run(Module , + ModuleAnalysisManager ) { + if (ColdType == PGOOptions::ColdFuncOpt::Default) +return PreservedAnalyses::all(); + ProfileSummaryInfo = AM.getResult(M); + FunctionAnalysisManager = + AM.getResult(M).getManager(); + bool MadeChange = false; + for (Function : M) { +if (F.isDeclaration()) + continue; +if (!shouldRunOnFunction(F, PSI, FAM)) + continue; +// Add optsize/minsize/optnone if requested. +switch (ColdType) { +case PGOOptions::ColdFuncOpt::Default: + llvm_unreachable("bailed out for default above"); + break; +case PGOOptions::ColdFuncOpt::OptSize: + if (!F.hasFnAttribute(Attribute::OptimizeNone) && + !F.hasFnAttribute(Attribute::OptimizeForSize) && + !F.hasFnAttribute(Attribute::MinSize)) { +F.addFnAttr(Attribute::OptimizeForSize); +MadeChange = true; + } + break; +case PGOOptions::ColdFuncOpt::MinSize: + // Change optsize to minsize. + if (!F.hasFnAttribute(Attribute::OptimizeNone) && + !F.hasFnAttribute(Attribute::MinSize)) { +F.removeFnAttr(Attribute::OptimizeForSize); aeubanks wrote: it was intentional for optnone to take precedence, but I've changed it to bail out if we see optnone/minsize/optsize at all https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: > FWIW we've tried this with sampling PGO in the past. While on paper this > seems like an obvious thing to do, in reality aggressively tuning down opt > level for cold functions can lead to regression since profile isn't always > accurate. > > That said, as long as this change only provides options for users to make > decision and not changing the default behavior, it's probably fine. IIUC, this won't affect sample profile unless you mark the sample profile as "accurate" (e.g. `-profile-sample-accurate`). But I should double check. https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: > How does this relate to the existing `shouldOptimizeForSize(Function&, ...)` > and `shouldOptimizeForSize(MachineFunction&, ...)` APIs which appear to > provide similar functionality at a first glance. If they are the same, then > we should have a plan in place to cleanup and only have one system > afterwards, if there are important differences, then I wouldn't mind some > comments explaining them. This is intended to replace `shouldOptimizeForSize()`. We've seen multiple cases of calls to `shouldOptimizeForSize()` blowing up compile times if we're not being careful with the calls to it, since it ends up calling expensive profile information code. The replacement is to just check if the function has the `optsize`/`minsize` attribute. I'll mention this in the description. The basic block versions of these should remain as they are since they actually do need to look at profile information to determine per-block information. https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Test ignored target-specific options for AMDGPU/NVPTX (PR #79222)
@@ -0,0 +1,7 @@ +/// Some target-specific options are ignored for GPU, so %clang exits with code 0. +// DEFINE: %{gpu_opts} = --cuda-gpu-arch=sm_60 --cuda-path=%S/Inputs/CUDA/usr/local/cuda --no-cuda-version-check aeubanks wrote: these defines seem overkill and harder to read compared to just duplicating the command line twice https://github.com/llvm/llvm-project/pull/79222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -1127,6 +1134,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); + if (EnableMarkColdFunctions && PGOOpt && + (PGOOpt->Action == PGOOptions::SampleUse || + PGOOpt->Action == PGOOptions::IRUse)) aeubanks wrote: actually based on the other comment to also apply to manually marked cold functions, now always run this regardless of `PGOOpt` https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -0,0 +1,65 @@ +//===--===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" +#include "llvm/Analysis/BlockFrequencyInfo.h" +#include "llvm/Analysis/ProfileSummaryInfo.h" +#include "llvm/IR/PassManager.h" + +using namespace llvm; + +PreservedAnalyses MarkColdFunctionsPass::run(Module , + ModuleAnalysisManager ) { + if (ColdType == PGOOptions::ColdFuncAttr::None) +return PreservedAnalyses::all(); + ProfileSummaryInfo = AM.getResult(M); + if (!PSI.hasProfileSummary()) +return PreservedAnalyses::all(); + FunctionAnalysisManager = + AM.getResult(M).getManager(); + bool MadeChange = false; + for (Function : M) { +if (F.isDeclaration()) + continue; +BlockFrequencyInfo = FAM.getResult(F); +if (!PSI.isFunctionColdInCallGraph(, BFI)) aeubanks wrote: done https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/69030 >From e52a811c3b643548837b4e630e8293a0b6857ad4 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 13 Oct 2023 14:40:28 -0700 Subject: [PATCH 1/5] [PGO] Add ability to mark cold functions as optsize/minsize/optnone The performance of cold functions shouldn't matter too much, so if we care about binary sizes, add an option to mark cold functions as optsize/minsize for binary size, or optnone for compile times [1]. Clang patch will be in a future patch Initial version: https://reviews.llvm.org/D149800 [1] https://discourse.llvm.org/t/rfc-new-feature-proposal-de-optimizing-cold-functions-using-pgo-info/56388 --- clang/lib/CodeGen/BackendUtil.cpp | 18 ++-- llvm/include/llvm/Support/PGOOptions.h| 3 + .../Instrumentation/MarkColdFunctions.h | 28 ++ llvm/lib/LTO/LTOBackend.cpp | 12 ++- llvm/lib/Passes/PassBuilder.cpp | 1 + llvm/lib/Passes/PassBuilderPipelines.cpp | 12 +++ llvm/lib/Passes/PassRegistry.def | 1 + llvm/lib/Support/PGOOptions.cpp | 7 +- .../Transforms/Instrumentation/CMakeLists.txt | 1 + .../Instrumentation/MarkColdFunctions.cpp | 65 + .../Transforms/MarkColdFunctions/basic.ll | 97 +++ llvm/tools/opt/NewPMDriver.cpp| 24 - .../lib/Transforms/Instrumentation/BUILD.gn | 1 + 13 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h create mode 100644 llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp create mode 100644 llvm/test/Transforms/MarkColdFunctions/basic.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index a6142d99f3b688d..7e9d3b8ea55a188 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -746,7 +746,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. @@ -755,28 +756,32 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PGOOpt = PGOOptions( CodeGenOpts.ProfileInstrumentUsePath, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling); +PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, -CodeGenOpts.PseudoProbeForProfiling); +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) // -fdebug-info-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, true); +PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, true); // Check to see if we want to generate a CS profile. if (CodeGenOpts.hasProfileCSIRInstr()) { @@ -799,7 +804,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", /*MemoryProfile=*/"", nullptr,
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -1127,6 +1134,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); + if (EnableMarkColdFunctions && PGOOpt && + (PGOOpt->Action == PGOOptions::SampleUse || + PGOOpt->Action == PGOOptions::IRUse)) aeubanks wrote: done https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
@@ -0,0 +1,28 @@ +//===- MarkColdFunctions.h - *- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H +#define LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H + +#include "llvm/IR/PassManager.h" +#include "llvm/Support/PGOOptions.h" + +namespace llvm { + +struct MarkColdFunctionsPass : public PassInfoMixin { aeubanks wrote: renamed https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/69030 >From e52a811c3b643548837b4e630e8293a0b6857ad4 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 13 Oct 2023 14:40:28 -0700 Subject: [PATCH 1/3] [PGO] Add ability to mark cold functions as optsize/minsize/optnone The performance of cold functions shouldn't matter too much, so if we care about binary sizes, add an option to mark cold functions as optsize/minsize for binary size, or optnone for compile times [1]. Clang patch will be in a future patch Initial version: https://reviews.llvm.org/D149800 [1] https://discourse.llvm.org/t/rfc-new-feature-proposal-de-optimizing-cold-functions-using-pgo-info/56388 --- clang/lib/CodeGen/BackendUtil.cpp | 18 ++-- llvm/include/llvm/Support/PGOOptions.h| 3 + .../Instrumentation/MarkColdFunctions.h | 28 ++ llvm/lib/LTO/LTOBackend.cpp | 12 ++- llvm/lib/Passes/PassBuilder.cpp | 1 + llvm/lib/Passes/PassBuilderPipelines.cpp | 12 +++ llvm/lib/Passes/PassRegistry.def | 1 + llvm/lib/Support/PGOOptions.cpp | 7 +- .../Transforms/Instrumentation/CMakeLists.txt | 1 + .../Instrumentation/MarkColdFunctions.cpp | 65 + .../Transforms/MarkColdFunctions/basic.ll | 97 +++ llvm/tools/opt/NewPMDriver.cpp| 24 - .../lib/Transforms/Instrumentation/BUILD.gn | 1 + 13 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h create mode 100644 llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp create mode 100644 llvm/test/Transforms/MarkColdFunctions/basic.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index a6142d99f3b688d..7e9d3b8ea55a188 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -746,7 +746,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. @@ -755,28 +756,32 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PGOOpt = PGOOptions( CodeGenOpts.ProfileInstrumentUsePath, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling); +PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, -CodeGenOpts.PseudoProbeForProfiling); +PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, +CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) // -fdebug-info-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, true); +PGOOptions::NoAction, PGOOptions::NoCSAction, +PGOOptions::ColdFuncAttr::None, true); // Check to see if we want to generate a CS profile. if (CodeGenOpts.hasProfileCSIRInstr()) { @@ -799,7 +804,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", /*MemoryProfile=*/"", nullptr,
[clang-tools-extra] [llvm] [clang] [CGProfile] Use callee's PGO name when caller->callee is an indirect call. (PR #78610)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/78610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [CGProfile] Use callee's PGO name when caller->callee is an indirect call. (PR #78610)
aeubanks wrote: I'm missing the full context behind this patch, but code seems reasonable https://github.com/llvm/llvm-project/pull/78610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CUDA, NVPTX] accept/ignore any -mcmodel arguments. (PR #70740)
aeubanks wrote: should this have had a test? I'm trying to do the same thing for `-mlarge-data-threshold` and am trying to find an appropriate place to add a test https://github.com/llvm/llvm-project/pull/70740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [lld] [lld/ELF] Hint if R_X86_64_PC32 overflows and references a SHF_X86_64_LARGE section (PR #73045)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/73045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [lld] [lld/ELF] Hint if R_X86_64_PC32 overflows and references a SHF_X86_64_LARGE section (PR #73045)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/73045 >From 0145020ef2a803ec797e42f95bacde05dc32eac1 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 21 Nov 2023 14:01:04 -0800 Subject: [PATCH 1/3] [lld/ELF] Hint if R_X86_64_PC32 overflows and references a SHF_X86_64_LARGE section Makes it clearer what the issue is when hand-written assembly doesn't follow medium code model assumptions in a medium code model build. Alternative to #71248 by only hinting on an overflow. --- lld/ELF/Relocations.cpp | 6 ++ lld/test/ELF/x86-64-pc32-overflow-large.s | 25 +++ 2 files changed, 31 insertions(+) create mode 100644 lld/test/ELF/x86-64-pc32-overflow-large.s diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index fe3d7f419e84aa6..37a2363094020d0 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -105,6 +105,12 @@ void elf::reportRangeError(uint8_t *loc, const Relocation , const Twine , hint = "; references '" + lld::toString(*rel.sym) + '\''; else if (auto *d = dyn_cast(rel.sym)) hint = ("; references section '" + d->section->name + "'").str(); + +if (rel.type == R_X86_64_PC32 && rel.sym->getOutputSection() && +(rel.sym->getOutputSection()->flags & SHF_X86_64_LARGE)) { + hint += "; a R_X86_64_PC32 relocation should not reference a section " + "marked SHF_X86_64_LARGE"; +} } if (!errPlace.srcLoc.empty()) hint += "\n>>> referenced by " + errPlace.srcLoc; diff --git a/lld/test/ELF/x86-64-pc32-overflow-large.s b/lld/test/ELF/x86-64-pc32-overflow-large.s new file mode 100644 index 000..54c20eddfd04c33 --- /dev/null +++ b/lld/test/ELF/x86-64-pc32-overflow-large.s @@ -0,0 +1,25 @@ +# REQUIRES: x86 +# RUN: split-file %s %t +# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/a.s -o %t/a.o +# RUN: not ld.lld %t/a.o -T %t/lds -o /dev/null 2>&1 | FileCheck %s + +# CHECK: error: {{.*}}a.o:(.text+{{.*}}): relocation R_X86_64_PC32 out of range: {{.*}}; a R_X86_64_PC32 relocation should not reference a section marked SHF_X86_64_LARGE + +#--- a.s +.text +.globl _start +.type _start, @function +_start: + movq hello(%rip), %rax + +.section ldata,"awl",@progbits +.type hello, @object +.globl hello +hello: +.long 1 + +#--- lds +SECTIONS { + .text 0x10 : { *(.text) } + ldata 0x8020 : { *(ldata) } +} >From a4432ade194df8dedb7b4990a29efaa4e822d486 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 17 Jan 2024 19:20:39 + Subject: [PATCH 2/3] check emachine --- lld/ELF/Relocations.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index e1e047c3d052427..10f62f21274239b 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -106,7 +106,8 @@ void elf::reportRangeError(uint8_t *loc, const Relocation , const Twine , else if (auto *d = dyn_cast(rel.sym)) hint = ("; references section '" + d->section->name + "'").str(); -if (rel.type == R_X86_64_PC32 && rel.sym->getOutputSection() && +if (config->emachine == EM_X86_64 && rel.type == R_X86_64_PC32 && +rel.sym->getOutputSection() && (rel.sym->getOutputSection()->flags & SHF_X86_64_LARGE)) { hint += "; a R_X86_64_PC32 relocation should not reference a section " "marked SHF_X86_64_LARGE"; >From 4447d474587ebf06d8b778616ef8c96c7cbd3c46 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 17 Jan 2024 23:29:55 + Subject: [PATCH 3/3] update error message --- lld/ELF/Relocations.cpp | 4 ++-- lld/test/ELF/x86-64-pc32-overflow-large.s | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index 10f62f21274239b..59b022079587175 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -109,8 +109,8 @@ void elf::reportRangeError(uint8_t *loc, const Relocation , const Twine , if (config->emachine == EM_X86_64 && rel.type == R_X86_64_PC32 && rel.sym->getOutputSection() && (rel.sym->getOutputSection()->flags & SHF_X86_64_LARGE)) { - hint += "; a R_X86_64_PC32 relocation should not reference a section " - "marked SHF_X86_64_LARGE"; + hint += "; R_X86_64_PC32 should not reference a section marked " + "SHF_X86_64_LARGE"; } } if (!errPlace.srcLoc.empty()) diff --git a/lld/test/ELF/x86-64-pc32-overflow-large.s b/lld/test/ELF/x86-64-pc32-overflow-large.s index 54c20eddfd04c33..fb8f3e4480c40f5 100644 --- a/lld/test/ELF/x86-64-pc32-overflow-large.s +++ b/lld/test/ELF/x86-64-pc32-overflow-large.s @@ -3,7 +3,7 @@ # RUN: llvm-mc -filetype=obj -triple=x86_64 %t/a.s -o %t/a.o # RUN: not ld.lld %t/a.o -T %t/lds -o /dev/null 2>&1 | FileCheck %s -# CHECK: error: {{.*}}a.o:(.text+{{.*}}): relocation R_X86_64_PC32 out of range: {{.*}}; a R_X86_64_PC32 relocation should not
[clang] [clang][Darwin] Remove legacy framework search path logic in the frontend (PR #75841)
aeubanks wrote: sorry, I keep missing notifications, will find a repro https://github.com/llvm/llvm-project/pull/75841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [lld] [clang-tools-extra] [lld/ELF] Hint if R_X86_64_PC32 overflows and references a SHF_X86_64_LARGE section (PR #73045)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/73045 >From 0145020ef2a803ec797e42f95bacde05dc32eac1 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 21 Nov 2023 14:01:04 -0800 Subject: [PATCH 1/2] [lld/ELF] Hint if R_X86_64_PC32 overflows and references a SHF_X86_64_LARGE section Makes it clearer what the issue is when hand-written assembly doesn't follow medium code model assumptions in a medium code model build. Alternative to #71248 by only hinting on an overflow. --- lld/ELF/Relocations.cpp | 6 ++ lld/test/ELF/x86-64-pc32-overflow-large.s | 25 +++ 2 files changed, 31 insertions(+) create mode 100644 lld/test/ELF/x86-64-pc32-overflow-large.s diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index fe3d7f419e84aa..37a2363094020d 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -105,6 +105,12 @@ void elf::reportRangeError(uint8_t *loc, const Relocation , const Twine , hint = "; references '" + lld::toString(*rel.sym) + '\''; else if (auto *d = dyn_cast(rel.sym)) hint = ("; references section '" + d->section->name + "'").str(); + +if (rel.type == R_X86_64_PC32 && rel.sym->getOutputSection() && +(rel.sym->getOutputSection()->flags & SHF_X86_64_LARGE)) { + hint += "; a R_X86_64_PC32 relocation should not reference a section " + "marked SHF_X86_64_LARGE"; +} } if (!errPlace.srcLoc.empty()) hint += "\n>>> referenced by " + errPlace.srcLoc; diff --git a/lld/test/ELF/x86-64-pc32-overflow-large.s b/lld/test/ELF/x86-64-pc32-overflow-large.s new file mode 100644 index 00..54c20eddfd04c3 --- /dev/null +++ b/lld/test/ELF/x86-64-pc32-overflow-large.s @@ -0,0 +1,25 @@ +# REQUIRES: x86 +# RUN: split-file %s %t +# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/a.s -o %t/a.o +# RUN: not ld.lld %t/a.o -T %t/lds -o /dev/null 2>&1 | FileCheck %s + +# CHECK: error: {{.*}}a.o:(.text+{{.*}}): relocation R_X86_64_PC32 out of range: {{.*}}; a R_X86_64_PC32 relocation should not reference a section marked SHF_X86_64_LARGE + +#--- a.s +.text +.globl _start +.type _start, @function +_start: + movq hello(%rip), %rax + +.section ldata,"awl",@progbits +.type hello, @object +.globl hello +hello: +.long 1 + +#--- lds +SECTIONS { + .text 0x10 : { *(.text) } + ldata 0x8020 : { *(ldata) } +} >From a4432ade194df8dedb7b4990a29efaa4e822d486 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 17 Jan 2024 19:20:39 + Subject: [PATCH 2/2] check emachine --- lld/ELF/Relocations.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index e1e047c3d05242..10f62f21274239 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -106,7 +106,8 @@ void elf::reportRangeError(uint8_t *loc, const Relocation , const Twine , else if (auto *d = dyn_cast(rel.sym)) hint = ("; references section '" + d->section->name + "'").str(); -if (rel.type == R_X86_64_PC32 && rel.sym->getOutputSection() && +if (config->emachine == EM_X86_64 && rel.type == R_X86_64_PC32 && +rel.sym->getOutputSection() && (rel.sym->getOutputSection()->flags & SHF_X86_64_LARGE)) { hint += "; a R_X86_64_PC32 relocation should not reference a section " "marked SHF_X86_64_LARGE"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust -mlarge-data-threshold handling (PR #77958)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/77958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust -mlarge-data-threshold handling (PR #77958)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/77958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Adjust -mlarge-data-threshold handling (PR #77958)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/77958 >From 3a54757173faffe07da55223c52621691afad54d Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 12 Jan 2024 18:13:06 + Subject: [PATCH 1/2] [clang] Adjust -mlarge-data-threshold handling Make it apply to x86-64 medium and large code models since that's what the backend does. Warn if it's used for x86-32. Default to 0, let the driver set it to 65536 for the medium code model if one is not passed. Set it to 0 for the large code model by default to match gcc and since some users make assumptions about the large code model that any small data will break. --- .../clang/Basic/DiagnosticDriverKinds.td | 2 +- clang/include/clang/Driver/Options.td | 2 +- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 27 --- clang/test/CodeGen/large-data-threshold.c | 5 +++- clang/test/Driver/large-data-threshold.c | 12 ++--- 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 0a8a77fadbeb1b..8b5232a6df3958 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -472,7 +472,7 @@ def warn_unsupported_branch_protection: Warning < def err_sls_hardening_arm_not_supported : Error< "-mharden-sls is only supported on armv7-a or later">; def warn_drv_large_data_threshold_invalid_code_model: Warning< - "'%0' only applies to medium code model">, + "'%0' only applies to medium and large code models">, InGroup; def note_drv_command_failed_diag_msg : Note< diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 7f97d6b6faa398..e8afa1ea4126d8 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4393,7 +4393,7 @@ def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group, MarshallingInfoString, [{"default"}]>; def mlarge_data_threshold_EQ : Joined<["-"], "mlarge-data-threshold=">, Group, Visibility<[ClangOption, CC1Option]>, - MarshallingInfoInt, "65535">; + MarshallingInfoInt, "0">; def mtls_size_EQ : Joined<["-"], "mtls-size=">, Group, Visibility<[ClangOption, CC1Option]>, HelpText<"Specify bit size of immediate TLS offsets (AArch64 ELF only): " diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index ad6fc71c1e5038..0cfe7a0133b7e3 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -1201,7 +1201,7 @@ void CodeGenModule::Release() { llvm::CodeModel::Model codeModel = static_cast(CM); getModule().setCodeModel(codeModel); - if (CM == llvm::CodeModel::Medium && + if ((CM == llvm::CodeModel::Medium || CM == llvm::CodeModel::Large) && Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86_64) { getModule().setLargeDataThreshold(getCodeGenOpts().LargeDataThreshold); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 1ee7ae602f3ce5..27a50b2c32e94d 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5752,20 +5752,29 @@ void Clang::ConstructJob(Compilation , const JobAction , } } - if (Arg *A = Args.getLastArg(options::OPT_mlarge_data_threshold_EQ)) { -if (!Triple.isX86()) { - D.Diag(diag::err_drv_unsupported_opt_for_target) - << A->getOption().getName() << TripleStr; -} else { - bool IsMediumCM = false; - if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) -IsMediumCM = StringRef(A->getValue()) == "medium"; - if (!IsMediumCM) { + if (Triple.getArch() == llvm::Triple::x86_64) { +bool IsMediumCM = false; +bool IsLargeCM = false; +if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) { + IsMediumCM = StringRef(A->getValue()) == "medium"; + IsLargeCM = StringRef(A->getValue()) == "large"; +} +if (Arg *A = Args.getLastArg(options::OPT_mlarge_data_threshold_EQ)) { + if (!IsMediumCM && !IsLargeCM) { D.Diag(diag::warn_drv_large_data_threshold_invalid_code_model) << A->getOption().getRenderName(); } else { A->render(Args, CmdArgs); } +} else if (IsMediumCM) { + CmdArgs.push_back("-mlarge-data-threshold=65536"); +} else if (IsLargeCM) { + CmdArgs.push_back("-mlarge-data-threshold=0"); +} + } else { +if (Arg *A = Args.getLastArg(options::OPT_mlarge_data_threshold_EQ)) { + D.Diag(diag::err_drv_unsupported_opt_for_target) + << A->getOption().getName() << TripleStr; } } diff --git a/clang/test/CodeGen/large-data-threshold.c b/clang/test/CodeGen/large-data-threshold.c index 29ae19e9b71899..d110ad2125c7bc 100644 ---
[clang] [clang] Adjust -mlarge-data-threshold handling (PR #77958)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/77958 Make it apply to x86-64 medium and large code models since that's what the backend does. Warn if it's used for x86-32. Default to 0, let the driver set it to 65536 for the medium code model if one is not passed. Set it to 0 for the large code model by default to match gcc and since some users make assumptions about the large code model that any small data will break. >From 3a54757173faffe07da55223c52621691afad54d Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 12 Jan 2024 18:13:06 + Subject: [PATCH] [clang] Adjust -mlarge-data-threshold handling Make it apply to x86-64 medium and large code models since that's what the backend does. Warn if it's used for x86-32. Default to 0, let the driver set it to 65536 for the medium code model if one is not passed. Set it to 0 for the large code model by default to match gcc and since some users make assumptions about the large code model that any small data will break. --- .../clang/Basic/DiagnosticDriverKinds.td | 2 +- clang/include/clang/Driver/Options.td | 2 +- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/lib/Driver/ToolChains/Clang.cpp | 27 --- clang/test/CodeGen/large-data-threshold.c | 5 +++- clang/test/Driver/large-data-threshold.c | 12 ++--- 6 files changed, 34 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 0a8a77fadbeb1b..8b5232a6df3958 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -472,7 +472,7 @@ def warn_unsupported_branch_protection: Warning < def err_sls_hardening_arm_not_supported : Error< "-mharden-sls is only supported on armv7-a or later">; def warn_drv_large_data_threshold_invalid_code_model: Warning< - "'%0' only applies to medium code model">, + "'%0' only applies to medium and large code models">, InGroup; def note_drv_command_failed_diag_msg : Note< diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 7f97d6b6faa398..e8afa1ea4126d8 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4393,7 +4393,7 @@ def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group, MarshallingInfoString, [{"default"}]>; def mlarge_data_threshold_EQ : Joined<["-"], "mlarge-data-threshold=">, Group, Visibility<[ClangOption, CC1Option]>, - MarshallingInfoInt, "65535">; + MarshallingInfoInt, "0">; def mtls_size_EQ : Joined<["-"], "mtls-size=">, Group, Visibility<[ClangOption, CC1Option]>, HelpText<"Specify bit size of immediate TLS offsets (AArch64 ELF only): " diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index ad6fc71c1e5038..0cfe7a0133b7e3 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -1201,7 +1201,7 @@ void CodeGenModule::Release() { llvm::CodeModel::Model codeModel = static_cast(CM); getModule().setCodeModel(codeModel); - if (CM == llvm::CodeModel::Medium && + if ((CM == llvm::CodeModel::Medium || CM == llvm::CodeModel::Large) && Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86_64) { getModule().setLargeDataThreshold(getCodeGenOpts().LargeDataThreshold); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 1ee7ae602f3ce5..27a50b2c32e94d 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5752,20 +5752,29 @@ void Clang::ConstructJob(Compilation , const JobAction , } } - if (Arg *A = Args.getLastArg(options::OPT_mlarge_data_threshold_EQ)) { -if (!Triple.isX86()) { - D.Diag(diag::err_drv_unsupported_opt_for_target) - << A->getOption().getName() << TripleStr; -} else { - bool IsMediumCM = false; - if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) -IsMediumCM = StringRef(A->getValue()) == "medium"; - if (!IsMediumCM) { + if (Triple.getArch() == llvm::Triple::x86_64) { +bool IsMediumCM = false; +bool IsLargeCM = false; +if (Arg *A = Args.getLastArg(options::OPT_mcmodel_EQ)) { + IsMediumCM = StringRef(A->getValue()) == "medium"; + IsLargeCM = StringRef(A->getValue()) == "large"; +} +if (Arg *A = Args.getLastArg(options::OPT_mlarge_data_threshold_EQ)) { + if (!IsMediumCM && !IsLargeCM) { D.Diag(diag::warn_drv_large_data_threshold_invalid_code_model) << A->getOption().getRenderName(); } else { A->render(Args, CmdArgs); } +} else if (IsMediumCM) { + CmdArgs.push_back("-mlarge-data-threshold=65536"); +} else if (IsLargeCM) { + CmdArgs.push_back("-mlarge-data-threshold=0"); +} + }
[clang] [clang][Darwin] Remove legacy framework search path logic in the frontend (PR #75841)
aeubanks wrote: is the clang invocation in the logs I posted sufficient? https://github.com/llvm/llvm-project/pull/75841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] New calling convention preserve_none (PR #76868)
aeubanks wrote: the clang changes should be split into a followup patch https://github.com/llvm/llvm-project/pull/76868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)
aeubanks wrote: ah it's because we something like ``` [cfi-unrelated-cast|cfi-derived-cast] src:*third_party/vulkan_memory_allocator/include/vk_mem_alloc.h ``` it seems like the new system doesn't match `[cfi-unrelated-cast|cfi-derived-cast]` https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [SpecialCaseList] Use glob by default (PR #74809)
aeubanks wrote: the file name is `vk_mem_alloc.h` so that shouldn't be the issue https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [SpecialCaseList] Use glob by default (PR #74809)
aeubanks wrote: This caused some ignorelist changes, e.g. `src:*third_party/vulkan_memory_allocator/include/vk_mem_alloc.h` didn't work anymore and the opt-out made it work again. Still investigating why. https://github.com/llvm/llvm-project/pull/74809 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits