Skip to content

[InstrProf][NFC] Refactor profdata trace tests #152550

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.

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
Aug 8, 2025

Conversation

ellishg
Copy link
Contributor

@ellishg ellishg commented Aug 7, 2025

Refactor some llvm-profdata tests to read text profiles which are easier to match with FileCheck

@llvmbot llvmbot added the PGO Profile Guided Optimizations label Aug 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2025

@llvm/pr-subscribers-pgo

Author: Ellis Hoag (ellishg)

Changes

Refactor some llvm-profdata tests to read text profiles which are easier to match with FileCheck


Full diff: https://github.com/llvm/llvm-project/pull/152550.diff

2 Files Affected:

  • (modified) llvm/test/tools/llvm-profdata/merge-traces.proftext (+24-21)
  • (modified) llvm/test/tools/llvm-profdata/read-traces.proftext (+9-12)
diff --git a/llvm/test/tools/llvm-profdata/merge-traces.proftext b/llvm/test/tools/llvm-profdata/merge-traces.proftext
index bcf29ba634ea6..f6b4b2cd51192 100644
--- a/llvm/test/tools/llvm-profdata/merge-traces.proftext
+++ b/llvm/test/tools/llvm-profdata/merge-traces.proftext
@@ -1,24 +1,27 @@
-# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s -o %t.profdata
-# RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=SAMPLE1,SEEN1
-# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %t.profdata -o %t.profdata
-# RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=SAMPLE2,SEEN2
-# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %t.profdata -o %t.profdata
-# RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=SAMPLE2,SEEN3
-# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %t.profdata -o %t.profdata
-# RUN: llvm-profdata show --temporal-profile-traces %t.profdata | FileCheck %s --check-prefixes=SAMPLE2,SEEN4
-
-# SEEN1: Temporal Profile Traces (samples=1 seen=1):
-# SEEN2: Temporal Profile Traces (samples=2 seen=2):
-# SEEN3: Temporal Profile Traces (samples=2 seen=3):
-# SEEN4: Temporal Profile Traces (samples=2 seen=4):
-# SAMPLE1: Temporal Profile Trace 0 (weight=1 count=3):
-# SAMPLE1:   a
-# SAMPLE1:   b
-# SAMPLE1:   c
-# SAMPLE2: Temporal Profile Trace 1 (weight=1 count=3):
-# SAMPLE2:   a
-# SAMPLE2:   b
-# SAMPLE2:   c
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s -o %t-2.profdata
+
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s --text | FileCheck %s --check-prefixes=CHECK,SEEN1,SAMPLE1
+
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s --text | FileCheck %s --check-prefixes=CHECK,SEEN2,SAMPLE2
+
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s %s --text | FileCheck %s --check-prefixes=CHECK,SEEN3,SAMPLE2
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %t-2.profdata %s --text | FileCheck %s --check-prefixes=CHECK,SEEN3,SAMPLE2
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %t-2.profdata --text | FileCheck %s --check-prefixes=CHECK,SEEN3,SAMPLE2
+
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s %s %s --text | FileCheck %s --check-prefixes=CHECK,SEEN4,SAMPLE2
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %t-2.profdata %s %s --text | FileCheck %s --check-prefixes=CHECK,SEEN4,SAMPLE2
+# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %t-2.profdata %t-2.profdata --text | FileCheck %s --check-prefixes=CHECK,SEEN4,SAMPLE2
+
+# CHECK: :temporal_prof_traces
+# CHECK: # Num Temporal Profile Traces:
+# SAMPLE1: 1
+# SAMPLE2: 2
+# CHECK: # Temporal Profile Trace Stream Size:
+# SEEN1: 1
+# SEEN2: 2
+# SEEN3: 3
+# SEEN4: 4
+# CHECK: a,b,c,
 
 # Header
 :ir
diff --git a/llvm/test/tools/llvm-profdata/read-traces.proftext b/llvm/test/tools/llvm-profdata/read-traces.proftext
index 87f69fe0d7610..5e822a9ea53ec 100644
--- a/llvm/test/tools/llvm-profdata/read-traces.proftext
+++ b/llvm/test/tools/llvm-profdata/read-traces.proftext
@@ -3,19 +3,16 @@
 # RUN: llvm-profdata merge -text %t.2.profdata -o %t.3.proftext
 # RUN: diff %t.1.proftext %t.3.proftext
 
-# RUN: llvm-profdata show --temporal-profile-traces %t.1.proftext | FileCheck %s
+# RUN: llvm-profdata merge -text %s | FileCheck %s
 
-# CHECK: Temporal Profile Traces (samples=3 seen=3):
-# CHECK: Temporal Profile Trace 0 (weight=1 count=3):
-# CHECK:   foo
-# CHECK:   bar
-# CHECK:   goo
-# CHECK: Temporal Profile Trace 1 (weight=3 count=3):
-# CHECK:   foo
-# CHECK:   goo
-# CHECK:   bar
-# CHECK: Temporal Profile Trace 2 (weight=1 count=1):
-# CHECK:   goo
+# CHECK:      :temporal_prof_traces
+# CHECK:      # Num Temporal Profile Traces:
+# CHECK-NEXT: 3
+# CHECK:      # Temporal Profile Trace Stream Size:
+# CHECK-NEXT: 3
+# CHECK-DAG:  foo,bar,goo,
+# CHECK-DAG:  foo,goo,bar,
+# CHECK-DAG:  goo,
 
 # Header
 :ir

Copy link
Contributor

@snehasish snehasish left a comment

Choose a reason for hiding this comment

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

lgtm

# SAMPLE2: a
# SAMPLE2: b
# SAMPLE2: c
# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s -o %t-2.profdata
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this closer to where t-2 is used?


# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s --text | FileCheck %s --check-prefixes=CHECK,SEEN1,SAMPLE1

# RUN: llvm-profdata merge --temporal-profile-trace-reservoir-size=2 %s %s --text | FileCheck %s --check-prefixes=CHECK,SEEN2,SAMPLE2
Copy link
Contributor

Choose a reason for hiding this comment

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

It was confusing to me to see the %s argument repeated. Maybe a comment to explain the intent might help?

@ellishg ellishg merged commit 3b32893 into llvm:main Aug 8, 2025
7 of 9 checks passed
@ellishg ellishg deleted the profdata-tests branch August 8, 2025 16:40
ellishg added a commit that referenced this pull request Aug 8, 2025
`InstrProfWriter::addTemporalProfileTraces()` did not correctly account
for when the sources traces are sampled, but the reservoir size is
larger than what it was before, meaning there is room for more traces.
Also, if the reservoir size decreased, meaning traces should be
truncated.

Depends on #152550 for the test
refactor
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Aug 8, 2025
`InstrProfWriter::addTemporalProfileTraces()` did not correctly account
for when the sources traces are sampled, but the reservoir size is
larger than what it was before, meaning there is room for more traces.
Also, if the reservoir size decreased, meaning traces should be
truncated.

Depends on llvm/llvm-project#152550 for the test
refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants