Skip to content

[WPD] Change Devirt Cutoff to use DebugCounter #170009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 1, 2025
+86 −21
Merged

[WPD] Change Devirt Cutoff to use DebugCounter #170009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d332ca
[WPD] Change Devirt Cutoff to use DebugCounter
boomanaiden154 Nov 29, 2025
c2c9e95
feedback
boomanaiden154 Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/DebugCounter.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
Expand Down Expand Up @@ -130,6 +131,8 @@ STATISTIC(NumUniqueRetVal, "Number of unique return value optimizations");
STATISTIC(NumVirtConstProp1Bit,
"Number of 1 bit virtual constant propagations");
STATISTIC(NumVirtConstProp, "Number of virtual constant propagations");
DEBUG_COUNTER(CallsToDevirt, "calls-to-devirt",
"Controls how many calls should be devirtualized.");

namespace llvm {

Expand Down Expand Up @@ -219,14 +222,6 @@ static cl::opt<bool> WholeProgramDevirtKeepUnreachableFunction(
cl::desc("Regard unreachable functions as possible devirtualize targets."),
cl::Hidden, cl::init(true));

/// If explicitly specified, the devirt module pass will stop transformation
/// once the total number of devirtualizations reach the cutoff value. Setting
/// this option to 0 explicitly will do 0 devirtualization.
static cl::opt<unsigned> WholeProgramDevirtCutoff(
"wholeprogramdevirt-cutoff",
cl::desc("Max number of devirtualizations for devirt module pass"),
cl::init(0));

/// Mechanism to add runtime checking of devirtualization decisions, optionally
/// trapping or falling back to indirect call on any that are not correct.
/// Trapping mode is useful for debugging undefined behavior leading to failures
Expand Down Expand Up @@ -377,9 +372,6 @@ VirtualCallTarget::VirtualCallTarget(GlobalValue *Fn, const TypeMemberInfo *TM)

namespace {

// Tracks the number of devirted calls in the IR transformation.
static unsigned NumDevirtCalls = 0;

// A slot in a set of virtual tables. The TypeID identifies the set of virtual
// tables, and the ByteOffset is the offset in bytes from the address point to
// the virtual function pointer.
Expand Down Expand Up @@ -1216,15 +1208,13 @@ void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
continue;

// Stop when the number of devirted calls reaches the cutoff.
if (WholeProgramDevirtCutoff.getNumOccurrences() > 0 &&
NumDevirtCalls >= WholeProgramDevirtCutoff)
return;
if (!DebugCounter::shouldExecute(CallsToDevirt))
continue;

if (RemarksEnabled)
VCallSite.emitRemark("single-impl",
TheFn->stripPointerCasts()->getName(), OREGetter);
NumSingleImpl++;
NumDevirtCalls++;
auto &CB = VCallSite.CB;
assert(!CB.getCalledFunction() && "devirtualizing direct call?");
IRBuilder<> Builder(&CB);
Expand Down
81 changes: 81 additions & 0 deletions llvm/test/Transforms/WholeProgramDevirt/calls-to-devirt.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
; REQUIRES: asserts

; Devirt calls debug counter is not explicitly set. Expect 3 remark messages.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import \
; RUN: -pass-remarks=wholeprogramdevirt \
; RUN: -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml \
; RUN: -print-debug-counter-queries < %s 2>&1 \
; RUN: | grep "remark" | count 3
; Devirt calls debug counter is set to 1. Expect one remark messages.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import \
; RUN: -pass-remarks=wholeprogramdevirt -debug-counter=calls-to-devirt=0 \
; RUN: -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml \
; RUN: -print-debug-counter-queries < %s 2>&1 \
; RUN: | FileCheck --check-prefix=CHECK-SINGLE %s
; Devirt calls debug counter is set outside the range of calls. Expect no remark message.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import \
; RUN: -pass-remarks=wholeprogramdevirt -debug-counter=calls-to-devirt=9999 \
; RUN: -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml \
; RUN: -print-debug-counter-queries < %s 2>&1 \
; RUN: | FileCheck -implicit-check-not="remark" --check-prefix=CHECK-NONE %s

; CHECK-SINGLE: DebugCounter calls-to-devirt=0 execute
; CHECK-SINGLE: remark
; CHECK-SINGLE-SAME: devirtualized a call
; CHECK-SINGLE: DebugCounter calls-to-devirt=1 skip
; CHECK-SINGLE: DebugCounter calls-to-devirt=2 skip

; CHECK-NONE: DebugCounter calls-to-devirt=0 skip
; CHECK-NONE: DebugCounter calls-to-devirt=1 skip
; CHECK-NONE: DebugCounter calls-to-devirt=2 skip

target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"

define i32 @call1(ptr %obj) #0 {
%vtable = load ptr, ptr %obj
%p = call i1 @llvm.type.test(ptr %vtable, metadata !"typeid1")
call void @llvm.assume(i1 %p)
%fptr = load ptr, ptr %vtable
%result = call i32 %fptr(ptr %obj, i32 1)
ret i32 %result
}

define i1 @call2(ptr %obj) #0 {
%vtable = load ptr, ptr %obj
%pair = call {ptr, i1} @llvm.type.checked.load(ptr %vtable, i32 8, metadata !"typeid2")
%fptr = extractvalue {ptr, i1} %pair, 0
%p = extractvalue {ptr, i1} %pair, 1
br i1 %p, label %cont, label %trap

cont:
%result = call i1 %fptr(ptr %obj, i32 undef)
ret i1 %result

trap:
call void @llvm.trap()
unreachable
}

define i1 @call3(ptr %obj) #0 {
%vtable = load ptr, ptr %obj
%pair = call {ptr, i1} @llvm.type.checked.load(ptr %vtable, i32 8, metadata !"typeid2")
%fptr = extractvalue {ptr, i1} %pair, 0
%p = extractvalue {ptr, i1} %pair, 1
br i1 %p, label %cont, label %trap

cont:
%result = call i1 %fptr(ptr %obj, i32 3)
ret i1 %result

trap:
call void @llvm.trap()
unreachable
}

declare void @llvm.assume(i1)
declare void @llvm.trap()
declare {ptr, i1} @llvm.type.checked.load(ptr, i32, metadata)
declare i1 @llvm.type.test(ptr, metadata)

attributes #0 = { "target-features"="+retpoline" }
6 changes: 0 additions & 6 deletions llvm/test/Transforms/WholeProgramDevirt/import.ll
View file
Open in desktop
Copy link
Contributor

@mingmingl-llvm mingmingl-llvm Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of

llvm-project/llvm/include/llvm/Support/DebugCounter.h

Lines 156 to 163 in 24b87b8

static bool isCountingEnabled() {
// Compile to nothing when debugging is off
#ifdef NDEBUG
return false;
#else
return instance().Enabled || instance().ShouldPrintCounter;
#endif
}

is that debug counter needs -DLLVM_ENABLE_ASSERTIONS=ON, and the test will need something like 'REQUIRES: asserts'.

Copy link
Contributor Author

@boomanaiden154 boomanaiden154 Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've split this into a separate test so that we can require assertions only where needed.

Copy link
Contributor

@teresajohnson teresajohnson Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that we can no longer use this option to bisect on a release compiler?

Copy link
Contributor

@mingmingl-llvm mingmingl-llvm Dec 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that we can no longer use this option to bisect on a release compiler?

We cannot bisect on a release compiler.

Fortunately, our internal infrastructure has a compiler that enables debug assertions and also supports enabling assertions on a per-file basis, which should help us work around the limitation internally.

Copy link
Contributor

@teresajohnson teresajohnson Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an alternative that would have maintained the ability to bisect with a release compiler? Could the counter have been moved into the DevirtModule class?

Copy link
Contributor

@nikic nikic Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably just make DebugCounters always available in release builds. I tried this a while ago (https://llvm-compile-time-tracker.com/compare.php?from=100a1d0c4caad0d0f2ec26b07d3cc73f59b9a9a8&to=341c03268229f2982df1fd7ddb5e3db7a2e7731a&stat=instructions:u) and while there is measurable overhead, it's probably acceptably small (and possibly optimizable).

Copy link
Contributor

@mingmingl-llvm mingmingl-llvm Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably just make DebugCounters always available in release builds.

This will help debugging a release-build compiler. Are you interested in driving this?

Is there an alternative that would have maintained the ability to bisect with a release compiler? Could the counter have been moved into the DevirtModule class?

If upstream is not ready to enable debug counters in a release build very soon, I can look into this.

Copy link
Contributor

@mingmingl-llvm mingmingl-llvm Dec 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I also followed up in the original RFC in https://discourse.llvm.org/t/rfc-reducing-process-creation-overhead-in-llvm-regression-tests/88612/14

Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-vcp-branch-funnel.yaml < %s | FileCheck --check-prefixes=CHECK,VCP,VCP-X86,VCP64,BRANCH-FUNNEL %s
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-branch-funnel.yaml < %s | FileCheck --check-prefixes=CHECK,BRANCH-FUNNEL,BRANCH-FUNNEL-NOVCP %s

; Cutoff value is not explicitly set. Expect 3 remark messages.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | grep "single-impl" | count 3
; Cutoff value is set to 1. Expect one remark messages.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-cutoff=1 -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | grep "single-impl" | count 1
; Cutoff value is explicitly set to zero. Expect no remark message.
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-cutoff=0 -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | FileCheck -implicit-check-not="remark" %s
target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"

Expand Down
Loading