[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)

2024-05-15 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-15 Thread Arthur Eubanks via cfe-commits

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)

2024-05-15 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-15 Thread Arthur Eubanks via cfe-commits

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)

2024-05-15 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-15 Thread Arthur Eubanks via cfe-commits

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)

2024-05-15 Thread Arthur Eubanks via cfe-commits

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)

2024-05-15 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-14 Thread Arthur Eubanks via cfe-commits

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)

2024-05-14 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-14 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-14 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-05-13 Thread Arthur Eubanks via cfe-commits

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)

2024-05-13 Thread Arthur Eubanks via cfe-commits

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)

2024-05-09 Thread Arthur Eubanks via cfe-commits

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)

2024-05-09 Thread Arthur Eubanks via cfe-commits

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)

2024-04-28 Thread Arthur Eubanks via cfe-commits

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)

2024-04-28 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-04-24 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-04-19 Thread Arthur Eubanks via cfe-commits

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)

2024-04-18 Thread Arthur Eubanks via cfe-commits

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)

2024-04-15 Thread Arthur Eubanks via cfe-commits

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)

2024-04-14 Thread Arthur Eubanks via cfe-commits

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)

2024-04-11 Thread Arthur Eubanks via cfe-commits

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)

2024-04-11 Thread Arthur Eubanks via cfe-commits

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"

2024-04-10 Thread Arthur Eubanks via cfe-commits

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)

2024-04-10 Thread Arthur Eubanks via cfe-commits

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)

2024-04-10 Thread Arthur Eubanks via cfe-commits

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)

2024-04-09 Thread Arthur Eubanks via cfe-commits

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)

2024-04-08 Thread Arthur Eubanks via cfe-commits

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)

2024-04-08 Thread Arthur Eubanks via cfe-commits

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)

2024-04-08 Thread Arthur Eubanks via cfe-commits

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)

2024-04-06 Thread Arthur Eubanks via cfe-commits

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)

2024-04-06 Thread Arthur Eubanks via cfe-commits

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)

2024-03-26 Thread Arthur Eubanks via cfe-commits

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)

2024-03-26 Thread Arthur Eubanks via cfe-commits

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)

2024-03-26 Thread Arthur Eubanks via cfe-commits

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)

2024-03-26 Thread Arthur Eubanks via cfe-commits

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)

2024-03-21 Thread Arthur Eubanks via cfe-commits

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)

2024-03-21 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-03-21 Thread Arthur Eubanks via cfe-commits

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)

2024-03-21 Thread Arthur Eubanks via cfe-commits

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)

2024-03-20 Thread Arthur Eubanks via cfe-commits

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)

2024-03-11 Thread Arthur Eubanks via cfe-commits

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)

2024-03-11 Thread Arthur Eubanks via cfe-commits

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)

2024-03-11 Thread Arthur Eubanks via cfe-commits

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)

2024-03-11 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-03-07 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-03-07 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-03-07 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-03-07 Thread Arthur Eubanks via cfe-commits

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)

2024-03-07 Thread Arthur Eubanks via cfe-commits

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)

2024-03-05 Thread Arthur Eubanks via cfe-commits

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)

2024-02-13 Thread Arthur Eubanks via cfe-commits

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)

2024-02-13 Thread Arthur Eubanks via cfe-commits

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)

2024-02-13 Thread Arthur Eubanks via cfe-commits

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)

2024-02-13 Thread Arthur Eubanks via cfe-commits

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)

2024-02-13 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-12 Thread Arthur Eubanks via cfe-commits

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)

2024-02-09 Thread Arthur Eubanks via cfe-commits

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)

2024-02-08 Thread Arthur Eubanks via cfe-commits

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)

2024-02-05 Thread Arthur Eubanks via cfe-commits

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)

2024-02-05 Thread Arthur Eubanks via cfe-commits

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)

2024-02-05 Thread Arthur Eubanks via cfe-commits

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)

2024-02-05 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-02-05 Thread Arthur Eubanks via cfe-commits

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)

2024-02-05 Thread Arthur Eubanks via cfe-commits

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)

2024-01-23 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-01-23 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-01-23 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-01-23 Thread Arthur Eubanks via cfe-commits

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)

2024-01-23 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-01-23 Thread Arthur Eubanks via cfe-commits


@@ -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)

2024-01-23 Thread Arthur Eubanks via cfe-commits

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)

2024-01-18 Thread Arthur Eubanks via cfe-commits

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)

2024-01-18 Thread Arthur Eubanks via cfe-commits

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)

2024-01-18 Thread Arthur Eubanks via cfe-commits

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)

2024-01-17 Thread Arthur Eubanks via cfe-commits

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)

2024-01-17 Thread Arthur Eubanks via cfe-commits

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)

2024-01-17 Thread Arthur Eubanks via cfe-commits

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)

2024-01-17 Thread Arthur Eubanks via cfe-commits

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)

2024-01-12 Thread Arthur Eubanks via cfe-commits

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)

2024-01-12 Thread Arthur Eubanks via cfe-commits

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)

2024-01-12 Thread Arthur Eubanks via cfe-commits

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)

2024-01-12 Thread Arthur Eubanks via cfe-commits

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)

2024-01-10 Thread Arthur Eubanks via cfe-commits

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)

2024-01-08 Thread Arthur Eubanks via cfe-commits

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)

2024-01-06 Thread Arthur Eubanks via cfe-commits

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)

2024-01-06 Thread Arthur Eubanks via cfe-commits

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)

2024-01-05 Thread Arthur Eubanks via cfe-commits

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


  1   2   3   4   >