[clang] [llvm] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
https://github.com/aeubanks closed 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] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
aeubanks wrote: thanks for doing 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] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
https://github.com/aeubanks approved this pull request. 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] [ConstantFold] Remove non-trivial gep-of-gep fold (PR #93823)
https://github.com/aeubanks approved this pull request. this PR looks good, and further dropping the fold is also good https://github.com/llvm/llvm-project/pull/93823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
aeubanks wrote: looks like `CodeGen/AMDGPU/llc-pipeline.ll` is failing 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] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
https://github.com/aeubanks approved this pull request. 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] [compiler-rt] [llvm] [mlir] [ConstantFold] Remove notional over-indexing fold (PR #93697)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/93697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
@@ -1030,6 +1036,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, Phase != ThinOrFullLTOPhase::ThinLTOPostLink) MPM.addPass(SampleProfileProbePass(TM)); + // Instrument function entry and exit before all inlining. + if (!isLTOPostLink(Phase)) { +MPM.addPass(createModuleToFunctionPassAdaptor( aeubanks wrote: if you have a small local repro, you can pass `-Wl,-mllvm,-print-after-all` to the link command to have it print IR after every pass and see where the calls are getting added. or to see changes in the pre-link optimization pipeline, `-mllvm=-print-after-all` (or `-mllvm=-print-changed=quiet`, which doesn't work with the IR parts of the codegen pipeline for *reasons*) 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] [EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines (PR #92171)
@@ -1030,6 +1036,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, Phase != ThinOrFullLTOPhase::ThinLTOPostLink) MPM.addPass(SampleProfileProbePass(TM)); + // Instrument function entry and exit before all inlining. + if (!isLTOPostLink(Phase)) { +MPM.addPass(createModuleToFunctionPassAdaptor( aeubanks wrote: the check for `Phase != ThinOrFullLTOPhase::FullLTOPostLink` is unnecessary, `buildModuleSimplificationPipeline` isn't called for FullLTO post link I'm not sure why you're seeing that behavior if we're only running the post-inline instrumenter once in the codegen pipeline 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] [lld] [llvm] Run ObjCContractPass in Default Codegen Pipeline (PR #92331)
@@ -31,6 +31,10 @@ ; CHECK-NEXT: AArch64 Stack Tagging ; CHECK-NEXT: SME ABI Pass ; CHECK-NEXT: Exception handling preparation +; CHECK-NEXT: Dominator Tree Construction +; CHECK-NEXT: Basic Alias Analysis (stateless AA impl) +; CHECK-NEXT: Function Alias Analysis Results aeubanks wrote: this is where having the new pass manager for codegen would be nice https://github.com/llvm/llvm-project/pull/92331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Move instrumentation passes (PR #92171)
@@ -1030,6 +1036,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, Phase != ThinOrFullLTOPhase::ThinLTOPostLink) MPM.addPass(SampleProfileProbePass(TM)); + // Instrument function entry and exit before all inlining. + if (!isLTOPostLink(Phase)) { +MPM.addPass(createModuleToFunctionPassAdaptor( aeubanks wrote: actually is it possible to add this into `EarlyFPM` below? it's nice to have fewer module->function adaptors. (e.g. in `EarlyFPM` we run all the function passes on a single function before moving to the next one which is nice for memory locality) sorry for suggesting this so late, since it'll require updating all the pipeline tests... but first check that none of the other tests fail 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] Move instrumentation passes (PR #92171)
@@ -1,5 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -debug-info-kind=standalone -emit-llvm -o - %s -finstrument-functions | FileCheck %s aeubanks wrote: this should also not be testing the pass and should have -disable-llvm-passes 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] Move instrumentation passes (PR #92171)
https://github.com/aeubanks commented: looks pretty good, can you update the commit title to something like `[EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines` I've added test coverage for mcount-aix in 3ec57a7ed60a19c5e3e18655e19c997a469d10ec, can you merge that in? 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] Move instrumentation passes (PR #92171)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Move instrumentation passes (PR #92171)
@@ -1,25 +1,13 @@ // RUN: %clang_cc1 -pg -triple powerpc-ibm-aix7.2.0.0 -emit-llvm %s -o - | FileCheck %s aeubanks wrote: also -disable-llvm-passes here 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)
@@ -1,37 +1,27 @@ // REQUIRES: x86-registered-target -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s __attribute__((always_inline)) int leaf(int x) { return x; // CHECK-LABEL: define {{.*}} @leaf -// CHECK: call void @__cyg_profile_func_enter -// CHECK-NOT: cyg_profile -// CHECK: call void @__cyg_profile_func_exit // CHECK-NOT: cyg_profile // CHECK: ret } int root(int x) { return leaf(x); // CHECK-LABEL: define {{.*}} @root -// CHECK: call void @__cyg_profile_func_enter -// CHECK-NOT: cyg_profile - -// Inlined from leaf(): -// CHECK: call void @__cyg_profile_func_enter -// CHECK-NOT: cyg_profile -// CHECK: call void @__cyg_profile_func_exit // CHECK-NOT: cyg_profile - -// CHECK: call void @__cyg_profile_func_exit // CHECK: ret // NOINLINE-LABEL: define {{.*}} @root -// NOINLINE: call void @__cyg_profile_func_enter -// NOINLINE-NOT: cyg_profile -// NOINLINE: call void @__cyg_profile_func_exit // NOINLINE-NOT: cyg_profile // NOINLINE: ret } + +// CHECK: attributes #0 = { alwaysinline {{.*}} "instrument-function-entry"="__cyg_profile_func_enter" "instrument-function-exit"="__cyg_profile_func_exit" +// CHECK: attributes #1 = { {{.*}} "instrument-function-entry"="__cyg_profile_func_enter" "instrument-function-exit"="__cyg_profile_func_exit" +// NOINLINE: attributes #0 = { alwaysinline {{.*}} "instrument-function-entry-inlined"="__cyg_profile_func_enter" "instrument-function-exit-inlined"="__cyg_profile_func_exit" aeubanks wrote: drop the `alwaysinline` check since that's not relevant (we can also remove it from the source), and check ` { {{.*}}"instrument-function-entry"=...` in case `"instrument-function-entry"` is the first attribute in the group and there's only one space before it 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)
@@ -1,37 +1,27 @@ // REQUIRES: x86-registered-target -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s __attribute__((always_inline)) int leaf(int x) { return x; // CHECK-LABEL: define {{.*}} @leaf -// CHECK: call void @__cyg_profile_func_enter -// CHECK-NOT: cyg_profile -// CHECK: call void @__cyg_profile_func_exit // CHECK-NOT: cyg_profile aeubanks wrote: no need to CHECK the IR here https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks commented: forgot to hit "submit review"... 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)
@@ -1,37 +1,27 @@ // REQUIRES: x86-registered-target -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O0 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions -O2 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -disable-llvm-passes -triple x86_64-unknown-unknown -finstrument-functions-after-inlining -O2 -o - -emit-llvm %s | FileCheck -check-prefix=NOINLINE %s __attribute__((always_inline)) int leaf(int x) { return x; // CHECK-LABEL: define {{.*}} @leaf aeubanks wrote: check `#0` here 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)
@@ -0,0 +1,29 @@ +; RUN: llc -mtriple=x86_64-- -O0 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-- -O1 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-- -O2 < %s | FileCheck %s + +; The codegen should insert post-inlining instrumentation calls and should not +; insert pre-inlining instrumentation. + +; CHECK-NOT: callq __cyg_profile_func_enter + +define void @leaf_function() #0 { +; CHECK-LABEL: leaf_function: +; CHECK: callq __cyg_profile_func_enter_bare +; CHECK: movqleaf_function@GOTPCREL(%rip), %rdi aeubanks wrote: checking the specifics of how the function is called shouldn't be in this test that tests the pipeline, the point of this test is just a quick end-to-end test testing we inserted instrumentation. if we want to test this we should have a separate test focused on the exact instruction sequence. but I don't think we need to add that test coverage in this PR so I'd remove the `%rdi` CHECK line 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)
@@ -0,0 +1,44 @@ +; RUN: opt -passes="default" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="default" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="thinlto-pre-link,thinlto" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="thinlto-pre-link" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="thinlto" -S < %s | FileCheck -check-prefix=LTO %s +; RUN: opt -passes="lto" -S < %s | FileCheck -check-prefix=LTO %s + +; Pre-inline instrumentation should be inserted, but not by LTO/ThinLTO passes. + +target triple = "x86_64-unknown-linux" + +define void @leaf_function() #0 { +entry: + ret void +; PRELTO-LABEL: entry: +; PRELTO-NEXT: %0 ={{( tail)?}} call ptr @llvm.returnaddress(i32 0) aeubanks wrote: no need to add `{{( tail)?}}` if it's at the very beginning. for this specific CHECK line I'd simplify to `{{.*}}` 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)
@@ -1,41 +0,0 @@ -// REQUIRES: arm-registered-target,aarch64-registered-target - -// RUN: %clang -target armv7-unknown-none-eabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang -target armv7-unknown-none-eabi -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang --target=aarch64-unknown-none-gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT -// RUN: %clang -target armv7-unknown-linux-gnueabi -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI -// RUN: %clang -target armv7-unknown-linux-gnueabi -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU -// RUN: %clang --target=aarch64-unknown-linux -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER -// RUN: %clang -target armv7-unknown-linux-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI -// RUN: %clang -target armv7-unknown-linux-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM-EABI-MEABI-GNU -// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER -// RUN: %clang -target armv7-unknown-freebsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER -// RUN: %clang --target=aarch64-unknown-freebsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-ARM64-EABI-FREEBSD -// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER -// RUN: %clang -target armv7-unknown-openbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER -// RUN: %clang --target=aarch64-unknown-openbsd -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix UNDER_UNDER -// RUN: %clang -target armv7-unknown-netbsd-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER -// RUN: %clang -target armv7-unknown-netbsd-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER -// RUN: %clang --target=aarch64-unknown-netbsd -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNDER_UNDER -// RUN: %clang -target armv7-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang -target armv7-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang -target arm64-apple-ios -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang -target arm64-apple-ios -pg -meabi gnu -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,UNSUPPORTED -// RUN: %clang -target armv7-unknown-rtems-gnueabihf -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT -// RUN: %clang -target armv7-unknown-rtems-gnueabihf -meabi gnu -pg -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,MCOUNT -// RUN: %clang --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT -// RUN: %clang --target=aarch64-unknown-rtems -pg -S -emit-llvm -o - %s | FileCheck %s -check-prefixes=CHECK,MCOUNT - -int f() { - return 0; -} - -// CHECK-LABEL: f -// TODO: add profiling support for arm-baremetal aeubanks wrote: we should keep both these tests you're deleting to ensure that we're emitting the correct function attributes. add `-Xclang -disable-llvm-passes` to the clang invocations to not run any passes and check the function attributes. this test is important to check that we're calling the correct `mcount`, and the other test is important to test that `-finstrument-functions-after-inlining` is emitting the correct function attribute (that test is the only instance of `-finstrument-functions-after-inlining`) 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)
@@ -0,0 +1,44 @@ +; RUN: opt -passes="default" -S < %s | FileCheck -check-prefix=PRELTO %s aeubanks wrote: `PRELTO` isn't quite accurate since we also instrument in the default pipelines, I'd use `INSTRUMENT`/`NOINSTRUMENT` or even just `YES`/`NO` :) 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)
@@ -0,0 +1,44 @@ +; RUN: opt -passes="default" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="default" -S < %s | FileCheck -check-prefix=PRELTO %s +; RUN: opt -passes="thinlto-pre-link,thinlto" -S < %s | FileCheck -check-prefix=PRELTO %s aeubanks wrote: don't run two pipelines in a single `opt` invocation, and we need tests for `thinlto`/`lto` 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] [lldb] [llvm] Remove some `try_compile` CMake checks for compiler flags (PR #92953)
https://github.com/aeubanks approved this pull request. awesome, thanks! https://github.com/llvm/llvm-project/pull/92953 ___ 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)
@@ -0,0 +1,29 @@ +; RUN: llc -mtriple=x86_64-- -O0 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-- -O1 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-- -O2 < %s | FileCheck %s + +; The codegen should insert post-inlining instrumentation calls and should not +; insert pre-inlining instrumentation. + +; CHECK-NOT: callq __cyg_profile_func_enter + +define void @leaf_function() #0 { +; CHECK-LABEL: leaf_function: +; CHECK: callq __cyg_profile_func_enter_bare +; CHECK: movqleaf_function@GOTPCREL(%rip), %rdi aeubanks wrote: probably don't need this CHECK line, it just makes the test more brittle 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)
@@ -0,0 +1,31 @@ +; RUN: opt -passes="default" -S < %s | FileCheck %s +; RUN: opt -passes="thinlto-pre-link" -S < %s | FileCheck %s +; RUN: opt -passes="thinlto-pre-link,thinlto" -S < %s | FileCheck %s + +target triple = "x86_64-unknown-linux" + +define void @leaf_function() #0 { aeubanks wrote: mark this function as `alwaysinline` so testing `-O0` works 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)
@@ -0,0 +1,31 @@ +; RUN: opt -passes="default" -S < %s | FileCheck %s aeubanks wrote: should also test `O0` versions of these, plus `lto-pre-link` 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)
@@ -0,0 +1,31 @@ +; RUN: opt -passes="default" -S < %s | FileCheck %s +; RUN: opt -passes="thinlto-pre-link" -S < %s | FileCheck %s +; RUN: opt -passes="thinlto-pre-link,thinlto" -S < %s | FileCheck %s aeubanks wrote: no need to run `thinlto` here, but we should test that `thinlto`/`lto` don't run this pass by adding different RUN lines with a different `FileCheck %s --checkprefix=NO` 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] [lld] [llvm] Run ObjCContractPass in Distributed Thin-LTO Pipeline (PR #92331)
aeubanks wrote: it seems like this should just be in the default codegen pipeline? you'd need to change the pass to bail out early if there are no relevant intrinsics (by checking if the module contains the intrinsic declaration) to not affect compile times https://github.com/llvm/llvm-project/pull/92331 ___ 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)
https://github.com/aeubanks approved this pull request. we've resolved the performance regression from the previous patch internally, thanks for waiting! 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] [IR] Add getelementptr nusw and nuw flags (PR #90824)
@@ -0,0 +1,86 @@ +//===-- llvm/GEPNoWrapFlags.h - NoWrap flags for GEPs ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file defines the nowrap flags for getelementptr operators. +// +//===--===// + +#ifndef LLVM_IR_GEPNOWRAPFLAGS_H +#define LLVM_IR_GEPNOWRAPFLAGS_H + +namespace llvm { + +class GEPNoWrapFlags { + enum : unsigned{ +InBoundsFlag = (1 << 0), +NUSWFlag = (1 << 1), +NUWFlag = (1 << 2), + }; + + unsigned Flags; + GEPNoWrapFlags(unsigned Flags) : Flags(Flags) { +assert((!isInBounds() || hasNoUnsignedSignedWrap()) && + "inbounds implies nusw"); + } + +public: + GEPNoWrapFlags() : Flags(0) {} + // For historical reasons, interpret plain boolean as InBounds. aeubanks wrote: TODO: remove? https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
https://github.com/aeubanks approved this pull request. thanks, I think abstracting out GEPNoWrapFlags is good https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
@@ -0,0 +1,86 @@ +//===-- llvm/GEPNoWrapFlags.h - NoWrap flags for GEPs ---*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// This file defines the nowrap flags for getelementptr operators. +// +//===--===// + +#ifndef LLVM_IR_GEPNOWRAPFLAGS_H +#define LLVM_IR_GEPNOWRAPFLAGS_H + +namespace llvm { + +class GEPNoWrapFlags { + enum : unsigned{ aeubanks wrote: clang-format https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Add getelementptr nusw and nuw flags (PR #90824)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/90824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -1028,6 +1029,14 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, Phase != ThinOrFullLTOPhase::ThinLTOPostLink) MPM.addPass(SampleProfileProbePass(TM)); + // Instrument function entry and exit before all inlining. + if (Phase != ThinOrFullLTOPhase::ThinLTOPostLink && + Phase != ThinOrFullLTOPhase::FullLTOPostLink && + Phase != ThinOrFullLTOPhase::None) { aeubanks wrote: we need this pass when `Phase == ThinOrFullLTOPhase::None` right? I'd extract out `isLTOPostLink` to mirror the existing `isLTOPreLink` https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks commented: looks pretty good to me for testing the pre-inliner one, we should add some tests in `llvm/test/Transforms/EntryExitInstrumenter/` that invoke things like `opt -passes='default'`, `opt -passes='thinlto-pre-link'`, `opt -passes='thinlto'` to make sure that the pass did/didn't insert the call given some IR with the appropriate function attribute for testing the post-inliner one, an `llc` x86-64 test that checks that a call to the function was generated in the assembly given some IR with the appropriate function attribute is enough https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -101,6 +101,7 @@ void initializeEarlyMachineLICMPass(PassRegistry&); void initializeEarlyTailDuplicatePass(PassRegistry&); void initializeEdgeBundlesPass(PassRegistry&); void initializeEHContGuardCatchretPass(PassRegistry &); +void initializeEntryExitInstrumenterPass(PassRegistry&); aeubanks wrote: not necessary https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
aeubanks wrote: can you add links to https://reviews.llvm.org/D97608, https://github.com/rust-lang/rust/issues/92109, https://github.com/llvm/llvm-project/issues/52853 https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -135,6 +138,65 @@ static bool runOnFunction(Function , bool PostInlining) { return Changed; } +namespace { +struct EntryExitInstrumenter : public FunctionPass { + static char ID; + EntryExitInstrumenter() : FunctionPass(ID) { +initializeEntryExitInstrumenterPass(*PassRegistry::getPassRegistry()); + } + void getAnalysisUsage(AnalysisUsage ) const override { +AU.addPreserved(); +AU.addPreserved(); + } + bool runOnFunction(Function ) override { return ::runOnFunction(F, false); } +}; +char EntryExitInstrumenter::ID = 0; + +struct PostInlineEntryExitInstrumenter : public FunctionPass { + static char ID; + PostInlineEntryExitInstrumenter() : FunctionPass(ID) { +initializePostInlineEntryExitInstrumenterPass( +*PassRegistry::getPassRegistry()); + } + void getAnalysisUsage(AnalysisUsage ) const override { +AU.addPreserved(); +AU.addPreserved(); + } + bool runOnFunction(Function ) override { return ::runOnFunction(F, true); } +}; +char PostInlineEntryExitInstrumenter::ID = 0; +} + +INITIALIZE_PASS_BEGIN( +EntryExitInstrumenter, "ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() (pre inlining)", +false, false) +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) +INITIALIZE_PASS_END( +EntryExitInstrumenter, "ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() (pre inlining)", +false, false) + +INITIALIZE_PASS_BEGIN( +PostInlineEntryExitInstrumenter, "post-inline-ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() " +"(post inlining)", +false, false) +INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass) +INITIALIZE_PASS_END( +PostInlineEntryExitInstrumenter, "post-inline-ee-instrument", +"Instrument function entry/exit with calls to e.g. mcount() " +"(post inlining)", +false, false) + +FunctionPass *llvm::createEntryExitInstrumenterPass() { aeubanks wrote: we don't need this one since we're adding it in the optimization pipeline which only works with the new pass manager https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -670,9 +670,6 @@ void CodeGenPassBuilder::addIRPasses( !Opt.DisablePartialLibcallInlining) addPass(PartiallyInlineLibCallsPass()); - // Instrument function entry and exit, e.g. with calls to mcount(). - addPass(EntryExitInstrumenterPass(/*PostInlining=*/true)); aeubanks wrote: don't touch this one, this is the WIP port of the codegen pipeline to the new pass manager (and this is actually what we want, that the codegen pipeline does this) https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] wip: Move instrumentation passes (PR #92171)
@@ -1016,6 +1000,11 @@ void EmitAssemblyHelper::RunOptimizationPipeline( if (!IsThinLTOPostLink) { addSanitizers(TargetTriple, CodeGenOpts, LangOpts, PB); addKCFIPass(TargetTriple, LangOpts, PB); + PB.registerPipelineStartEPCallback( aeubanks wrote: we should be consistent and make both entry exit instrumenters part of the default pipelines, not a clang-specific add-on this would probably be somewhere in `buildModuleSimplificationPipeline`, checking `Phase` (and make sure -O0 works by modifying `buildO0DefaultPipeline`) https://github.com/llvm/llvm-project/pull/92171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Revert "Generate assume loads only with -fstrict-vtable-pointers" (PR #91900)
aeubanks wrote: > -fstrict-vtable-pointers IS experimental, but if you recall, this particular > optimization was added to -fstrict-vtable-pointers because of the effects it > had on compile-time, not because of correctness issues. can you clarify what you mean by "this particular optimization"? you mean adding or not adding assume loads? when I said "regress", I meant runtime performance, not compile times, I should have been clearer https://github.com/llvm/llvm-project/pull/91900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CodeGen] Revert "Generate assume loads only with -fstrict-vtable-pointers" (PR #91900)
aeubanks wrote: adding assumes in general has issues: https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 do you have proof that this change helps binaries and doesn't regress things? I have a feeling this will regress many things. `-fstrict-vtable-pointers` is still somewhat experimental https://github.com/llvm/llvm-project/pull/91900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (#91498)" (PR #91620)
aeubanks wrote: should be fine to revert as much as you want in a single PR, just make sure to mention what you're reverting in the description https://github.com/llvm/llvm-project/pull/91620 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix lookup of dependent operator= outside of complete-class contexts (PR #91498)
aeubanks wrote: Chromium is also seeing similar breakages. @sdkrystian is this breaking valid code? I can't tell from your latest comment. (if it is breaking valid code we should revert) https://github.com/llvm/llvm-project/pull/91498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
aeubanks wrote: btw we're still looking into a performance regression caused by #68882 that still repros with LLVM head, even after the SROA enhancements. this patch looks good, but can we hold off a bit on submitting this? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
@@ -944,43 +943,18 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP, return ConstantExpr::getIntToPtr(C, ResTy); } - // Otherwise form a regular getelementptr. Recompute the indices so that - // we eliminate over-indexing of the notional static type array bounds. - // This makes it easy to determine if the getelementptr is "inbounds". - - // For GEPs of GlobalValues, use the value type, otherwise use an i8 GEP. - if (auto *GV = dyn_cast(Ptr)) -SrcElemTy = GV->getValueType(); - else -SrcElemTy = Type::getInt8Ty(Ptr->getContext()); - - if (!SrcElemTy->isSized()) -return nullptr; - - Type *ElemTy = SrcElemTy; - SmallVector Indices = DL.getGEPIndicesForOffset(ElemTy, Offset); - if (Offset != 0) -return nullptr; - - // Try to add additional zero indices to reach the desired result element - // type. - // TODO: Should we avoid extra zero indices if ResElemTy can't be reached and - // we'll have to insert a bitcast anyway? - while (ElemTy != ResElemTy) { -Type *NextTy = GetElementPtrInst::getTypeAtIndex(ElemTy, (uint64_t)0); -if (!NextTy) - break; - -Indices.push_back(APInt::getZero(isa(ElemTy) ? 32 : BitWidth)); -ElemTy = NextTy; + // Try to infer inbounds for GEPs of globals. + if (!InBounds && Offset.isNonNegative()) { aeubanks wrote: is there a test for the case where `!Offset.isNonNegative()`? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ConstantFolding] Canonicalize constexpr GEPs to i8 (PR #89872)
@@ -944,43 +943,18 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP, return ConstantExpr::getIntToPtr(C, ResTy); } - // Otherwise form a regular getelementptr. Recompute the indices so that - // we eliminate over-indexing of the notional static type array bounds. - // This makes it easy to determine if the getelementptr is "inbounds". - - // For GEPs of GlobalValues, use the value type, otherwise use an i8 GEP. - if (auto *GV = dyn_cast(Ptr)) -SrcElemTy = GV->getValueType(); - else -SrcElemTy = Type::getInt8Ty(Ptr->getContext()); - - if (!SrcElemTy->isSized()) -return nullptr; - - Type *ElemTy = SrcElemTy; - SmallVector Indices = DL.getGEPIndicesForOffset(ElemTy, Offset); - if (Offset != 0) -return nullptr; - - // Try to add additional zero indices to reach the desired result element - // type. - // TODO: Should we avoid extra zero indices if ResElemTy can't be reached and - // we'll have to insert a bitcast anyway? - while (ElemTy != ResElemTy) { -Type *NextTy = GetElementPtrInst::getTypeAtIndex(ElemTy, (uint64_t)0); -if (!NextTy) - break; - -Indices.push_back(APInt::getZero(isa(ElemTy) ? 32 : BitWidth)); -ElemTy = NextTy; + // Try to infer inbounds for GEPs of globals. + if (!InBounds && Offset.isNonNegative()) { +bool CanBeNull, CanBeFreed; +uint64_t DerefBytes = +Ptr->getPointerDereferenceableBytes(DL, CanBeNull, CanBeFreed); +InBounds = DerefBytes != 0 && !CanBeNull && Offset.sle(DerefBytes); aeubanks wrote: can we remove the other constant folding? https://github.com/llvm/llvm-project/pull/89872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add flag to experiment with cold function attributes (PR #89298)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/89298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add flag to experiment with cold function attributes (PR #89298)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/89298 To be removed and promoted to a proper driver flag if experiments turn out fruitful. For now, this can be experimented with `-mllvm -pgo-cold-func-opt=[optsize|minsize|optnone|default] -mllvm -enable-pgo-force-function-attrs`. Original LLVM patch for this functionality: #69030 >From 1c69510ab5a92998cc443100ecfb551776fc03a0 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 15 Apr 2024 20:40:43 + Subject: [PATCH] [clang] Add flag to experiment with cold function attributes To be removed and promoted to a proper driver flag if experiments turn out fruitful. For now, this can be experimented with `-mllvm -pgo-cold-func-opt=[optsize|minsize|optnone|default] -mllvm -enable-pgo-force-function-attrs`. Original LLVM patch for this functionality: #69030 --- clang/lib/CodeGen/BackendUtil.cpp | 57 --- .../test/CodeGen/pgo-force-function-attrs.ll | 12 2 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 clang/test/CodeGen/pgo-force-function-attrs.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 6cc00b85664f41..22c3f8642ad8eb 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -104,6 +104,21 @@ static cl::opt ClSanitizeOnOptimizerEarlyEP( "sanitizer-early-opt-ep", cl::Optional, cl::desc("Insert sanitizers on OptimizerEarlyEP.")); +// Experiment to mark cold functions as optsize/minsize/optnone. +// TODO: remove once this is exposed as a proper driver flag. +static cl::opt ClPGOColdFuncAttr( +"pgo-cold-func-opt", cl::init(PGOOptions::ColdFuncOpt::Default), cl::Hidden, +cl::desc( +"Function attribute to apply to cold functions as determined by PGO"), +cl::values(clEnumValN(PGOOptions::ColdFuncOpt::Default, "default", + "Default (no attribute)"), + clEnumValN(PGOOptions::ColdFuncOpt::OptSize, "optsize", + "Mark cold functions with optsize."), + clEnumValN(PGOOptions::ColdFuncOpt::MinSize, "minsize", + "Mark cold functions with minsize."), + clEnumValN(PGOOptions::ColdFuncOpt::OptNone, "optnone", + "Mark cold functions with optnone."))); + extern cl::opt ProfileCorrelate; // Re-link builtin bitcodes after optimization @@ -768,42 +783,41 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. auto CSAction = CodeGenOpts.hasProfileCSIRUse() ? PGOOptions::CSIRUse : PGOOptions::NoCSAction; -PGOOpt = PGOOptions( -CodeGenOpts.ProfileInstrumentUsePath, "", -CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +PGOOpt = PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "", +CodeGenOpts.ProfileRemappingFile, +CodeGenOpts.MemoryProfileUsePath, VFS, +PGOOptions::IRUse, CSAction, ClPGOColdFuncAttr, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling -PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, -
[clang] [clang] Add flag to experiment with cold function attributes (PR #88793)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/88793 To be removed and promoted to a proper driver flag if experiments turn out fruitful. Original LLVM patch for this functionality: #69030 >From 52cd9974be908bf693832012e56e945e9e34f389 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 15 Apr 2024 20:40:43 + Subject: [PATCH] [clang] Add flag to experiment with cold function attributes To be removed and promoted to a proper driver flag if experiments turn out fruitful. Original LLVM patch for this functionality: #69030 --- clang/lib/CodeGen/BackendUtil.cpp | 57 +++ 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 6cc00b85664f41..22c3f8642ad8eb 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -104,6 +104,21 @@ static cl::opt ClSanitizeOnOptimizerEarlyEP( "sanitizer-early-opt-ep", cl::Optional, cl::desc("Insert sanitizers on OptimizerEarlyEP.")); +// Experiment to mark cold functions as optsize/minsize/optnone. +// TODO: remove once this is exposed as a proper driver flag. +static cl::opt ClPGOColdFuncAttr( +"pgo-cold-func-opt", cl::init(PGOOptions::ColdFuncOpt::Default), cl::Hidden, +cl::desc( +"Function attribute to apply to cold functions as determined by PGO"), +cl::values(clEnumValN(PGOOptions::ColdFuncOpt::Default, "default", + "Default (no attribute)"), + clEnumValN(PGOOptions::ColdFuncOpt::OptSize, "optsize", + "Mark cold functions with optsize."), + clEnumValN(PGOOptions::ColdFuncOpt::MinSize, "minsize", + "Mark cold functions with minsize."), + clEnumValN(PGOOptions::ColdFuncOpt::OptNone, "optnone", + "Mark cold functions with optnone."))); + extern cl::opt ProfileCorrelate; // Re-link builtin bitcodes after optimization @@ -768,42 +783,41 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. auto CSAction = CodeGenOpts.hasProfileCSIRUse() ? PGOOptions::CSIRUse : PGOOptions::NoCSAction; -PGOOpt = PGOOptions( -CodeGenOpts.ProfileInstrumentUsePath, "", -CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, -PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +PGOOpt = PGOOptions(CodeGenOpts.ProfileInstrumentUsePath, "", +CodeGenOpts.ProfileRemappingFile, +CodeGenOpts.MemoryProfileUsePath, VFS, +PGOOptions::IRUse, CSAction, ClPGOColdFuncAttr, +CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, -PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, +PGOOptions::NoCSAction, ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling); +ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling -PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, -PGOOptions::NoAction, PGOOptions::NoCSAction, -PGOOptions::ColdFuncOpt::Default, -CodeGenOpts.DebugInfoForProfiling, true); +PGOOpt = +PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, + PGOOptions::NoAction, PGOOptions::NoCSAction, + ClPGOColdFuncAttr, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) //
[clang] [flang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #88661)
https://github.com/aeubanks approved this pull request. lg, but update the commit message `Pull Request: https://github.com/llvm/llvm-project/pull/87866`, that's obsolete https://github.com/llvm/llvm-project/pull/88661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: yeah that patch makes those test pass with this PR, lgtm (you could also test locally by touching the files I mentioned above, e.g. even just `touch lib/clang/19/lib/linux/libclang_rt.builtins-aarch64-android.a` repro'd the test failure on my machine) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] be10070 - Revert "[Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin"
Author: Arthur Eubanks Date: 2024-04-10T23:41:51Z New Revision: be10070f91b86a6f126d2451852242bfcb2cd366 URL: https://github.com/llvm/llvm-project/commit/be10070f91b86a6f126d2451852242bfcb2cd366 DIFF: https://github.com/llvm/llvm-project/commit/be10070f91b86a6f126d2451852242bfcb2cd366.diff LOG: Revert "[Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin" This reverts commit ccdebbae4d77d3efc236af92c22941de5d437e01. Causes test failures in the presence of Android runtime libraries in resource-dir. See comments on https://github.com/llvm/llvm-project/pull/87866. Added: Modified: clang/lib/Driver/ToolChain.cpp clang/test/Driver/arm-compiler-rt.c clang/test/Driver/cl-link.c clang/test/Driver/compiler-rt-unwind.c clang/test/Driver/coverage-ld.c clang/test/Driver/instrprof-ld.c clang/test/Driver/linux-ld.c clang/test/Driver/mingw-sanitizers.c clang/test/Driver/msp430-toolchain.c clang/test/Driver/print-libgcc-file-name-clangrt.c clang/test/Driver/print-runtime-dir.c clang/test/Driver/riscv32-toolchain-extra.c clang/test/Driver/riscv32-toolchain.c clang/test/Driver/riscv64-toolchain-extra.c clang/test/Driver/riscv64-toolchain.c clang/test/Driver/sanitizer-ld.c clang/test/Driver/wasm-toolchain.c clang/test/Driver/wasm-toolchain.cpp clang/test/Driver/windows-cross.c clang/test/Driver/zos-ld.c flang/test/Driver/msvc-dependent-lib-flags.f90 Removed: diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 237092ed07e5dc..03450fc0f57b93 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -796,13 +796,7 @@ ToolChain::getTargetSubDirPath(StringRef BaseDir) const { std::optional ToolChain::getRuntimePath() const { SmallString<128> P(D.ResourceDir); llvm::sys::path::append(P, "lib"); - if (auto Ret = getTargetSubDirPath(P)) -return Ret; - // Darwin does not use per-target runtime directory. - if (Triple.isOSDarwin()) -return {}; - llvm::sys::path::append(P, Triple.str()); - return std::string(P); + return getTargetSubDirPath(P); } std::optional ToolChain::getStdlibPath() const { diff --git a/clang/test/Driver/arm-compiler-rt.c b/clang/test/Driver/arm-compiler-rt.c index cb6c29f48a7814..5e9e528400d08e 100644 --- a/clang/test/Driver/arm-compiler-rt.c +++ b/clang/test/Driver/arm-compiler-rt.c @@ -10,47 +10,47 @@ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABI -// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a" // RUN: %clang -target arm-linux-gnueabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABI-ABI -// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins-armhf.a" // RUN: %clang -target arm-linux-gnueabihf \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF -// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins-armhf.a" // RUN: %clang -target arm-linux-gnueabihf \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -mfloat-abi=soft -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF-ABI -// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a" // RUN: %clang -target arm-windows-itanium \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-WINDOWS -// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins.lib" +// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins-arm.lib" // RUN: %clang -target arm-linux-androideabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \ // RUN: -rtlib=compiler-rt -### %s 2>&1 \ // RUN: | FileCheck %s -check-prefix ARM-ANDROID -// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins.a" +// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins-arm-android.a" // RUN: not %clang --target=arm-linux-androideabi \ // RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \ // RUN:
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: it seems that the test depends on if certain android runtime libraries are present or not in the resource dir (without per-target runtime directory since that's still an issue on Android). perhaps the tests need `-resource-dir` to make them more hermetic? anyway, will revert, feel free to reland once the tests are fixed. this is what's in my resource dir if it's helpful for reproducing ``` $ ls lib/clang/19/lib/linux/ aarch64 libclang_rt.asan_cxx-aarch64-android.a libclang_rt.builtins-aarch64-android.a libclang_rt.tsan_cxx-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.so libclang_rt.asan-aarch64-android.a libclang_rt.asan-preinit-aarch64-android.a libclang_rt.profile-aarch64-android.a libclang_rt.ubsan_minimal-aarch64-android.a libclang_rt.ubsan_standalone_cxx-aarch64-android.a libclang_rt.asan-aarch64-android.so libclang_rt.asan_static-aarch64-android.a libclang_rt.tsan-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.a ``` https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: this seemes to be causing some test failures for us: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8751043232043110529/+/u/package_clang/stdout?format=raw ``` TEST 'Clang :: Driver/linux-ld.c' FAILED RUN: at line 92: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/linux-ld.c:102:25: error: CHECK-LD-RT-ANDROID: expected string not found in input // CHECK-LD-RT-ANDROID: libclang_rt.builtins.a" ^ :6:331: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" ^ :6:866: note: possible intended match here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" TEST 'Clang :: Driver/sanitizer-ld.c' FAILED RUN: at line 177: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/sanitizer-ld.c:187:24: error: CHECK-ASAN-ANDROID: expected string not found in input // CHECK-ASAN-ANDROID: libclang_rt.asan.so" ^ :6:320: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-pie" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan-arm-android.so" "--whole-archive" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan_static-arm-android.a" "--no-whole-archive" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/sanitizer-ld-d1ddb3.o"
[clang] Reland "[Win32][ELF] Make CodeView a DebugInfoFormat only for COFF format" (PR #87987)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/87987 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
aeubanks wrote: > I'd suggest adding bitcode upgrade if it isn't too hard (I don't think it > should be?) done https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/87906 >From 7a9df42b4c4f4f1b02dc3158d24800f3d4b68d8f Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Sun, 7 Apr 2024 05:29:36 + Subject: [PATCH 1/2] [clang][llvm] Remove "implicit-section-name" attribute D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. --- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/test/CodeGen/clang-sections-attribute.c | 3 --- clang/test/CodeGenCXX/clang-sections.cpp | 18 ++--- llvm/lib/CodeGen/TargetInstrInfo.cpp | 3 +-- .../CodeGen/TargetLoweringObjectFileImpl.cpp | 11 +--- llvm/lib/Target/TargetLoweringObjectFile.cpp | 5 .../CodeGen/AArch64/clang-section-macho.ll| 22 --- llvm/test/CodeGen/ARM/clang-section.ll| 24 - .../Generic/machine-function-splitter.ll | 27 +++ .../basic-block-sections-pragma-sections.ll | 4 +-- 10 files changed, 15 insertions(+), 104 deletions(-) delete mode 100644 llvm/test/CodeGen/AArch64/clang-section-macho.ll diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 00b3bfcaa0bc25..f4dbfe7a21f83c 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2627,7 +2627,7 @@ void CodeGenModule::setNonAliasAttributes(GlobalDecl GD, addUsedGlobal(F); if (auto *SA = D->getAttr()) if (!D->getAttr()) - F->addFnAttr("implicit-section-name", SA->getName()); + F->setSection(SA->getName()); llvm::AttrBuilder Attrs(F->getContext()); if (GetCPUAndFeaturesAttributes(GD, Attrs)) { diff --git a/clang/test/CodeGen/clang-sections-attribute.c b/clang/test/CodeGen/clang-sections-attribute.c index 70ed24ed07a280..768bdd4d87649e 100644 --- a/clang/test/CodeGen/clang-sections-attribute.c +++ b/clang/test/CodeGen/clang-sections-attribute.c @@ -69,8 +69,5 @@ static int int_zvar; // CHECK: define internal void @int_fun() #0 section ".int_fun_attr" // CHECK: define internal void @int_fun2() #0 section ".int_fun2_attr" // -// Function attributes should not include implicit-section-name. -// CHECK-NOT: attributes #0 = {{.*}}implicit-section-name -// // No other attribute group should be present in the file. // CHECK-NOT: attributes #1 diff --git a/clang/test/CodeGenCXX/clang-sections.cpp b/clang/test/CodeGenCXX/clang-sections.cpp index a444f2d0cae59c..aa159e552b1b3c 100644 --- a/clang/test/CodeGenCXX/clang-sections.cpp +++ b/clang/test/CodeGenCXX/clang-sections.cpp @@ -81,24 +81,22 @@ int hoo(void) { //CHECK: @p ={{.*}} constant i32 7, align 4 //CHECK: @_ZL5fptrs = internal constant [2 x ptr] [ptr @foo, ptr @goo], align {{4|8}} #3 -//CHECK: define{{.*}} i32 @foo() #5 { -//CHECK: define{{.*}} i32 @goo() #6 { -//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #7 -//CHECK: define{{.*}} i32 @hoo() #8 { +//ELF: define{{.*}} i32 @foo(){{.*}} section "my_text.1" { +//ELF: define{{.*}} i32 @goo(){{.*}} section "my_text.2" { +//MACHO: define{{.*}} i32 @foo(){{.*}} section "__TEXT,__mytext1" { +//MACHO: define{{.*}} i32 @goo(){{.*}} section "__TEXT,__mytext2" { + +// ensure zoo/hoo don't have a section +//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #6{{$}} +//CHECK: define{{.*}} i32 @hoo() #5 { //ELF: attributes #0 = { "bss-section"="my_bss.1" "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #1 = { "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #2 = { "bss-section"="my_bss.2" "rodata-section"="my_rodata.1" } //ELF: attributes #3 = { "bss-section"="my_bss.2" "data-section"="my_data.2" "relro-section"="my_relro.2" "rodata-section"="my_rodata.2" } //ELF: attributes #4 = { "relro-section"="my_relro.2" } -//ELF: attributes #5 = { {{.*"implicit-section-name"="my_text.1".*}} } -//ELF: attributes #6 = { {{.*"implicit-section-name"="my_text.2".*}} } //MACHO: attributes #0 = { "bss-section"="__BSS,__mybss1" "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #1 = { "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #2 = { "bss-section"="__BSS,__mybss2" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #3 = { "bss-section"="__BSS,__mybss2" "data-section"="__DATA,__mydata2" "relro-section"="__RELRO,__myrelro2" "rodata-section"="__RODATA,__myrodata2" } //MACHO: attributes #4 = { "relro-section"="__RELRO,__myrelro2" } -//MACHO: attributes #5 = { {{.*"implicit-section-name"="__TEXT,__mytext1".*}} } -//MACHO: attributes #6 = {
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
aeubanks wrote: this probably needs bitcode upgrade? https://github.com/llvm/llvm-project/pull/87906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][llvm] Remove "implicit-section-name" attribute (PR #87906)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/87906 D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. >From 7a9df42b4c4f4f1b02dc3158d24800f3d4b68d8f Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Sun, 7 Apr 2024 05:29:36 + Subject: [PATCH] [clang][llvm] Remove "implicit-section-name" attribute D33412/D33413 introduced this to support a clang pragma to set section names for a symbol depending on if it would be placed in bss/data/rodata/text, which may not be known until the backend. However, for text we know that only functions will go there, so just directly set the section in clang instead of going through a completely separate attribute. --- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/test/CodeGen/clang-sections-attribute.c | 3 --- clang/test/CodeGenCXX/clang-sections.cpp | 18 ++--- llvm/lib/CodeGen/TargetInstrInfo.cpp | 3 +-- .../CodeGen/TargetLoweringObjectFileImpl.cpp | 11 +--- llvm/lib/Target/TargetLoweringObjectFile.cpp | 5 .../CodeGen/AArch64/clang-section-macho.ll| 22 --- llvm/test/CodeGen/ARM/clang-section.ll| 24 - .../Generic/machine-function-splitter.ll | 27 +++ .../basic-block-sections-pragma-sections.ll | 4 +-- 10 files changed, 15 insertions(+), 104 deletions(-) delete mode 100644 llvm/test/CodeGen/AArch64/clang-section-macho.ll diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 00b3bfcaa0bc25..f4dbfe7a21f83c 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2627,7 +2627,7 @@ void CodeGenModule::setNonAliasAttributes(GlobalDecl GD, addUsedGlobal(F); if (auto *SA = D->getAttr()) if (!D->getAttr()) - F->addFnAttr("implicit-section-name", SA->getName()); + F->setSection(SA->getName()); llvm::AttrBuilder Attrs(F->getContext()); if (GetCPUAndFeaturesAttributes(GD, Attrs)) { diff --git a/clang/test/CodeGen/clang-sections-attribute.c b/clang/test/CodeGen/clang-sections-attribute.c index 70ed24ed07a280..768bdd4d87649e 100644 --- a/clang/test/CodeGen/clang-sections-attribute.c +++ b/clang/test/CodeGen/clang-sections-attribute.c @@ -69,8 +69,5 @@ static int int_zvar; // CHECK: define internal void @int_fun() #0 section ".int_fun_attr" // CHECK: define internal void @int_fun2() #0 section ".int_fun2_attr" // -// Function attributes should not include implicit-section-name. -// CHECK-NOT: attributes #0 = {{.*}}implicit-section-name -// // No other attribute group should be present in the file. // CHECK-NOT: attributes #1 diff --git a/clang/test/CodeGenCXX/clang-sections.cpp b/clang/test/CodeGenCXX/clang-sections.cpp index a444f2d0cae59c..aa159e552b1b3c 100644 --- a/clang/test/CodeGenCXX/clang-sections.cpp +++ b/clang/test/CodeGenCXX/clang-sections.cpp @@ -81,24 +81,22 @@ int hoo(void) { //CHECK: @p ={{.*}} constant i32 7, align 4 //CHECK: @_ZL5fptrs = internal constant [2 x ptr] [ptr @foo, ptr @goo], align {{4|8}} #3 -//CHECK: define{{.*}} i32 @foo() #5 { -//CHECK: define{{.*}} i32 @goo() #6 { -//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #7 -//CHECK: define{{.*}} i32 @hoo() #8 { +//ELF: define{{.*}} i32 @foo(){{.*}} section "my_text.1" { +//ELF: define{{.*}} i32 @goo(){{.*}} section "my_text.2" { +//MACHO: define{{.*}} i32 @foo(){{.*}} section "__TEXT,__mytext1" { +//MACHO: define{{.*}} i32 @goo(){{.*}} section "__TEXT,__mytext2" { + +// ensure zoo/hoo don't have a section +//CHECK: declare i32 @zoo(ptr noundef, ptr noundef) #6{{$}} +//CHECK: define{{.*}} i32 @hoo() #5 { //ELF: attributes #0 = { "bss-section"="my_bss.1" "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #1 = { "data-section"="my_data.1" "rodata-section"="my_rodata.1" } //ELF: attributes #2 = { "bss-section"="my_bss.2" "rodata-section"="my_rodata.1" } //ELF: attributes #3 = { "bss-section"="my_bss.2" "data-section"="my_data.2" "relro-section"="my_relro.2" "rodata-section"="my_rodata.2" } //ELF: attributes #4 = { "relro-section"="my_relro.2" } -//ELF: attributes #5 = { {{.*"implicit-section-name"="my_text.1".*}} } -//ELF: attributes #6 = { {{.*"implicit-section-name"="my_text.2".*}} } //MACHO: attributes #0 = { "bss-section"="__BSS,__mybss1" "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #1 = { "data-section"="__DATA,__mydata1" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #2 = { "bss-section"="__BSS,__mybss2" "rodata-section"="__RODATA,__myrodata1" } //MACHO: attributes #3 = {
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: ah I see. I feel like this should be a more principled approach that other sanitizers also share, as you've mentioned as an alternative. do people not care about other sanitizers in production? (I'm going to be OOO for a week, so someone else will need to review) https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: > We already have similar stuff: > > ``` > if (PGOOpt && Phase != ThinOrFullLTOPhase::ThinLTOPostLink && > !PGOOpt->MemoryProfile.empty()) > MPM.addPass(MemProfUsePass(PGOOpt->MemoryProfile, PGOOpt->FS)); > ``` checking for ThinLTO pre/post link is a correctness thing though I think I'm still confused on exactly what the use case is and why we can't just ask the user to not specify hwasan in the PGO instrumented build. Just for user convenience? Or does clang change the emitted IR when hwasan is enabled? And that's what will lead to mismatched profiles? https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: > > why can't hwasan and PGO instrumentation coexist? > > They can, but binary is like 5x times slower, on top of 10x slowdown of PGO > instrumentation. (don't quote me on these numbers, they are from large but > single benchmark, still it's very slow) > If it's usable as a configuration, I don't see why we should prevent this. It still may be useful to some people. Seems like this checking should be done at a build system level if you don't want some codebase to compile with this configuration. > > and this seems like it should be an error at the clang driver level, > > instead of silently turning off one of the requested features > > 1. We need -fsanitizer=hwaddress, for attributes and profile matching, and > some special handling done in earlier passes. Do you mean that if you want a hwasan/PGO optimized build, you want the corresponding PGO instrumented build to also use hwasan? Doesn't PGO instrumentation/use happen before the sanitizer passes run? > 2. We don't wan't users care about profile instrumentation/use difference. https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [hwasan] Don't instrument when PGO profile is collected (PR #86739)
aeubanks wrote: why can't hwasan and PGO instrumentation coexist? and this seems like it should be an error at the clang driver level, instead of silently turning off one of the requested features https://github.com/llvm/llvm-project/pull/86739 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
@@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept aeubanks wrote: done https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/86039 >From bba8e4003c4ccc36497e62ad1696197e6987525c Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 20 Mar 2024 23:36:35 + Subject: [PATCH 1/2] [clang][SPIRV] Ignore -mcmodel The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- clang/test/Driver/unsupported-option-gpu.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 055884d275ce1b..035bfa35299756 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept // whatever works for the host. Ok = true; diff --git a/clang/test/Driver/unsupported-option-gpu.c b/clang/test/Driver/unsupported-option-gpu.c index f23cb71ebfb08e..5618b2cba72e16 100644 --- a/clang/test/Driver/unsupported-option-gpu.c +++ b/clang/test/Driver/unsupported-option-gpu.c @@ -2,4 +2,5 @@ // DEFINE: %{check} = %clang -### --target=x86_64-linux-gnu -c -mcmodel=medium // RUN: %{check} -x cuda %s --cuda-path=%S/Inputs/CUDA/usr/local/cuda --offload-arch=sm_60 --no-cuda-version-check -fbasic-block-sections=all +// RUN: %{check} -x hip %s --offload=spirv64 -nogpulib -nogpuinc // RUN: %{check} -x hip %s --rocm-path=%S/Inputs/rocm -nogpulib -nogpuinc >From 650c120c5ad8360124b4d45a90974f4d60622455 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Thu, 21 Mar 2024 21:44:55 + Subject: [PATCH 2/2] update comment --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 035bfa35299756..57ab8b6e91826c 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5805,7 +5805,7 @@ void Clang::ConstructJob(Compilation , const JobAction , Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); } else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { - // NVPTX/AMDGPU does not care about the code model and will accept + // NVPTX/AMDGPU/SPIRV does not care about the code model and will accept // whatever works for the host. Ok = true; } else if (Triple.isSPARC64()) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Don't warn on -mcmodel (PR #86039)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/86039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][SPIRV] Ignore -mcmodel (PR #86039)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/86039 The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. >From bba8e4003c4ccc36497e62ad1696197e6987525c Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Wed, 20 Mar 2024 23:36:35 + Subject: [PATCH] [clang][SPIRV] Ignore -mcmodel The code model doesn't affect the sub-compilation, so don't check it. Followup to #70740. --- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- clang/test/Driver/unsupported-option-gpu.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 055884d275ce1b..035bfa35299756 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5804,7 +5804,7 @@ void Clang::ConstructJob(Compilation , const JobAction , } else if (Triple.getArch() == llvm::Triple::x86_64) { Ok = llvm::is_contained({"small", "kernel", "medium", "large", "tiny"}, CM); -} else if (Triple.isNVPTX() || Triple.isAMDGPU()) { +} else if (Triple.isNVPTX() || Triple.isAMDGPU() || Triple.isSPIRV()) { // NVPTX/AMDGPU does not care about the code model and will accept // whatever works for the host. Ok = true; diff --git a/clang/test/Driver/unsupported-option-gpu.c b/clang/test/Driver/unsupported-option-gpu.c index f23cb71ebfb08e..5618b2cba72e16 100644 --- a/clang/test/Driver/unsupported-option-gpu.c +++ b/clang/test/Driver/unsupported-option-gpu.c @@ -2,4 +2,5 @@ // DEFINE: %{check} = %clang -### --target=x86_64-linux-gnu -c -mcmodel=medium // RUN: %{check} -x cuda %s --cuda-path=%S/Inputs/CUDA/usr/local/cuda --offload-arch=sm_60 --no-cuda-version-check -fbasic-block-sections=all +// RUN: %{check} -x hip %s --offload=spirv64 -nogpulib -nogpuinc // RUN: %{check} -x hip %s --rocm-path=%S/Inputs/rocm -nogpulib -nogpuinc ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: > > > yes, but I'd like to that after we collect feedback from first users > > > > > > They are introduced by earlier transformations > > Note: I'd like to have special intrinsic for this optimization. When we > > have it, we likely don't need this SimplifyCFG. > > lgtm with a comment added on why we're adding the extra SimplifyCFG oh I missed that you'd added one. but maybe a TODO with your comment about an intrinsic? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: > > yes, but I'd like to that after we collect feedback from first users > > They are introduced by earlier transformations > > Note: I'd like to have special intrinsic for this optimization. When we have > it, we likely don't need this SimplifyCFG. lgtm with a comment added on why we're adding the extra SimplifyCFG https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: why isn't an earlier SimplifyCFG run in the function simplification pipeline handling these? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: profile matching happens right before the inliner/function simplification pipeline (after the initial `EarlyFPM` and `GlobalCleanupPM` cleanup) https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: I think this can go at the end of the function simplification pipeline (and there's already a SimplifyCFG run right after) with `registerScalarOptimizerLateEPCallback`. The function should already have been mostly simplified at that point. https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
@@ -744,6 +750,21 @@ static void addSanitizers(const Triple , // LastEP does not need GlobalsAA. PB.registerOptimizerLastEPCallback(SanitizersCallback); } + + if (ClRemoveTraps) { +// We can optimize after inliner, and PGO profile matching. The hook below +// is called from `buildModuleOptimizationPipeline` just after profile use, +// and inliner is a part of `buildModuleSimplificationPipeline`, which is +// before `buildModuleOptimizationPipeline`. +PB.registerOptimizerEarlyEPCallback([&](ModulePassManager , aeubanks wrote: unnecessary `&` https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [IR] Change representation of getelementptr inrange (PR #84341)
aeubanks wrote: the only use of this is GlobalSplit, and it cares about ranges relative to the GlobalVariable, so if we're not planning on using this in more cases then I'd say relative to the source pointer makes sense. not sure if inrange would ever be useful for more than GlobalSplit, like being able to tighten the possible values of a load from a const array if we know the load is in some small range of the array, which doesn't seem super useful https://github.com/llvm/llvm-project/pull/84341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add optional pass to remove UBSAN traps using PGO (PR #84214)
aeubanks wrote: is there a long term plan to add a driver flag for this? https://github.com/llvm/llvm-project/pull/84214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [APINotes] Upstream Sema logic to apply API Notes to decls (PR #78445)
aeubanks wrote: > Oh, thanks @nikic for that data point. Let me try to avoid the overhead, I'll > put up a patch tomorrow morning. Did this ever happen? https://github.com/llvm/llvm-project/pull/78445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
aeubanks wrote: Sorry, I thought I had waited long enough and that the previous comments were addressed. Will address your comments in a follow-up. > This is good to go only because it's off by default. Otherwise it's not. > Sample PGO profile has inline context, so in the profile, we may have foo as > cold and bar->foo as hot, but if later inliner rejects bar->foo inlining, foo > can be hot. So marking foo as cold pre-inline can still be inaccurate (and > not conservative). Is this the main objection? I didn't understand this sentence the first time reading it. The Sample PGO inliner runs before this pass so it shouldn't be affected, as mentioned before. By "later inliner" do you mean the normal CGSCC inliner? Is it possible to have a call site be hot but the callee be cold? I don't currently have a plan to make this pass do anything by default, but I would like to resolve objections anyway. Using the `optsize` setting for this pass seems like it could be the default perhaps further in the future. https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
aeubanks wrote: for my information, which version of Visual Studio are you using? https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang] Remove #undef alloca workaround" (PR #81649)
https://github.com/aeubanks approved this pull request. https://github.com/llvm/llvm-project/pull/81649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO] Add ability to mark cold functions as optsize/minsize/optnone (PR #69030)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/69030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca workaround (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
https://github.com/aeubanks edited https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove old Linux kernel workaround for ensuring stack space (PR #81533)
https://github.com/aeubanks closed https://github.com/llvm/llvm-project/pull/81533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
aeubanks wrote: seeing if Windows CI catches anything https://github.com/llvm/llvm-project/pull/81534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove #undef alloca (PR #81534)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/81534 Added in 26670dcba1609574cba5942aff78ff97b567c5f3. >From c659a573a066809473ebb36421e612dcdcda5aef Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 12 Feb 2024 21:01:39 + Subject: [PATCH] [clang] Remove #undef alloca Added in 26670dcba1609574cba5942aff78ff97b567c5f3. --- clang/include/clang/Basic/Builtins.h | 4 1 file changed, 4 deletions(-) diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h index f955d21169556a..6700d1903a0088 100644 --- a/clang/include/clang/Basic/Builtins.h +++ b/clang/include/clang/Basic/Builtins.h @@ -20,10 +20,6 @@ #include "llvm/ADT/StringRef.h" #include -// VC++ defines 'alloca' as an object-like macro, which interferes with our -// builtins. -#undef alloca - namespace clang { class TargetInfo; class IdentifierTable; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove old Linux kernel workaround for ensuring stack space (PR #81533)
https://github.com/aeubanks created https://github.com/llvm/llvm-project/pull/81533 PR #71709 broke the Linux PIE build with `undefined symbol: alloca` errors. With the newly included `clang/Basic/Builtins.h` in that PR, it surfaces an issue with a combination of two previous patches. 26670dcba1609574cba5942aff78ff97b567c5f3 added `#undef alloca` so clang builtins handling of alloca would work under MSVC (unsure if this is still necessary). 194b6a3b1b1a99cc3c12c466a04320f271ebd8aa added code that calls `alloca` to workaround a Linux kernel < 4.1 bug. Given that Linux 4.1 was EOL in 2018, it should be ok to remove this workaround. >From 3dd69256a0a3f9cfabac54cabad5b3dcc2410c36 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 12 Feb 2024 20:51:37 + Subject: [PATCH] [clang] Remove old Linux kernel workaround for ensuring stack space PR #71709 broke the Linux PIE build with `undefined symbol: alloca` errors. With the newly included `clang/Basic/Builtins.h` in that PR, it surfaces an issue with a combination of two previous patches. 26670dcba1609574cba5942aff78ff97b567c5f3 added `#undef alloca` so clang builtins handling of alloca would work under MSVC (unsure if this is still necessary). 194b6a3b1b1a99cc3c12c466a04320f271ebd8aa added code that calls `alloca` to workaround a Linux kernel < 4.1 bug. Given that Linux 4.1 was EOL in 2018, it should be ok to remove this workaround. --- clang/tools/driver/cc1_main.cpp | 62 - 1 file changed, 62 deletions(-) diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp index e9d2c6aad371db..b5c6be3c557bb3 100644 --- a/clang/tools/driver/cc1_main.cpp +++ b/clang/tools/driver/cc1_main.cpp @@ -78,64 +78,6 @@ static void LLVMErrorHandler(void *UserData, const char *Message, } #ifdef CLANG_HAVE_RLIMITS -#if defined(__linux__) && defined(__PIE__) -static size_t getCurrentStackAllocation() { - // If we can't compute the current stack usage, allow for 512K of command - // line arguments and environment. - size_t Usage = 512 * 1024; - if (FILE *StatFile = fopen("/proc/self/stat", "r")) { -// We assume that the stack extends from its current address to the end of -// the environment space. In reality, there is another string literal (the -// program name) after the environment, but this is close enough (we only -// need to be within 100K or so). -unsigned long StackPtr, EnvEnd; -// Disable silly GCC -Wformat warning that complains about length -// modifiers on ignored format specifiers. We want to retain these -// for documentation purposes even though they have no effect. -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wformat" -#endif -if (fscanf(StatFile, - "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*lu %*lu %*lu %*lu %*lu " - "%*lu %*ld %*ld %*ld %*ld %*ld %*ld %*llu %*lu %*ld %*lu %*lu " - "%*lu %*lu %lu %*lu %*lu %*lu %*lu %*lu %*llu %*lu %*lu %*d %*d " - "%*u %*u %*llu %*lu %*ld %*lu %*lu %*lu %*lu %*lu %*lu %lu %*d", - , ) == 2) { -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic pop -#endif - Usage = StackPtr < EnvEnd ? EnvEnd - StackPtr : StackPtr - EnvEnd; -} -fclose(StatFile); - } - return Usage; -} - -#include - -LLVM_ATTRIBUTE_NOINLINE -static void ensureStackAddressSpace() { - // Linux kernels prior to 4.1 will sometimes locate the heap of a PIE binary - // relatively close to the stack (they are only guaranteed to be 128MiB - // apart). This results in crashes if we happen to heap-allocate more than - // 128MiB before we reach our stack high-water mark. - // - // To avoid these crashes, ensure that we have sufficient virtual memory - // pages allocated before we start running. - size_t Curr = getCurrentStackAllocation(); - const int kTargetStack = DesiredStackSize - 256 * 1024; - if (Curr < kTargetStack) { -volatile char *volatile Alloc = -static_cast(alloca(kTargetStack - Curr)); -Alloc[0] = 0; -Alloc[kTargetStack - Curr - 1] = 0; - } -} -#else -static void ensureStackAddressSpace() {} -#endif - /// Attempt to ensure that we have at least 8MiB of usable stack space. static void ensureSufficientStack() { struct rlimit rlim; @@ -159,10 +101,6 @@ static void ensureSufficientStack() { rlim.rlim_cur != DesiredStackSize) return; } - - // We should now have a stack of size at least DesiredStackSize. Ensure - // that we can actually use that much, if necessary. - ensureStackAddressSpace(); } #else static void ensureSufficientStack() {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor `IdentifierInfo::ObjcOrBuiltinID` (PR #71709)
aeubanks wrote: https://github.com/llvm/llvm-project/issues/4885 for why `#undef alloca` was added https://github.com/llvm/llvm-project/pull/71709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor `IdentifierInfo::ObjcOrBuiltinID` (PR #71709)
aeubanks wrote: I'll send out a PR to remove that code, and potentially also remove the `#undef alloca` separately https://github.com/llvm/llvm-project/pull/71709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits