Skip to content

Conversation

@cmtice
Copy link
Contributor

@cmtice cmtice commented Sep 10, 2025

As part of our migration for making lit internal shell be the default, this updates binary-id-offset.c to no longer require shell (at the cost of duplicating some code).

As part of our migration for making lit internal shell be the
default, this updates binary-id-offset.c to no longer require
shell (at the cost of duplicating some code).
@llvmbot llvmbot added compiler-rt PGO Profile Guided Optimizations labels Sep 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2025

@llvm/pr-subscribers-pgo

Author: None (cmtice)

Changes

As part of our migration for making lit internal shell be the default, this updates binary-id-offset.c to no longer require shell (at the cost of duplicating some code).


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

1 Files Affected:

  • (modified) compiler-rt/test/profile/Linux/binary-id-offset.c (+12-10)
diff --git a/compiler-rt/test/profile/Linux/binary-id-offset.c b/compiler-rt/test/profile/Linux/binary-id-offset.c
index dd4ce7dc4d03d..e42b992126ddd 100644
--- a/compiler-rt/test/profile/Linux/binary-id-offset.c
+++ b/compiler-rt/test/profile/Linux/binary-id-offset.c
@@ -5,28 +5,30 @@
 // (The DYN case would also apply to libraries, not explicitly tested here.)
 
 // DEFINE: %{cflags} =
-// DEFINE: %{check} = (                                                           \
-// DEFINE:     %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}   && \
-// DEFINE:     env LLVM_PROFILE_FILE=%t.profraw %run %t                        && \
-// DEFINE:     llvm-readelf --notes %t                                         && \
-// DEFINE:     llvm-profdata show --binary-ids %t.profraw                         \
-// DEFINE:   ) | FileCheck %s
 
 // REDEFINE: %{cflags} = -no-pie
-// RUN: %{check}
+// RUN: %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-readelf --notes %t && llvm-profdata show --binary-ids %t.profraw > FileCheck
 
 // REDEFINE: %{cflags} = -pie -fPIE
-// RUN: %{check}
+// RUN: %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-readelf --notes %t && llvm-profdata show --binary-ids %t.profraw > FileCheck
 
 // Moving the note after .bss also gives it extra LOAD segment padding,
 // making its memory offset different than its file offset.
 // RUN: echo "SECTIONS { .note.gnu.build-id : {} } INSERT AFTER .bss;" >%t.script
 
 // REDEFINE: %{cflags} = -no-pie -Wl,--script=%t.script
-// RUN: %{check}
+// RUN: %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-readelf --notes %t && llvm-profdata show --binary-ids %t.profraw > FileCheck
 
 // REDEFINE: %{cflags} = -pie -fPIE -Wl,--script=%t.script
-// RUN: %{check}
+// RUN: %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-readelf --notes %t && llvm-profdata show --binary-ids %t.profraw > FileCheck
 
 // CHECK-LABEL{LITERAL}: .note.gnu.build-id
 // CHECK: Build ID: [[ID:[0-9a-f]+]]

@cmtice cmtice requested review from boomanaiden154, cuviper and fmayer and removed request for cuviper September 10, 2025 20:53
// RUN: %{check}
// RUN: %clang_profgen -fuse-ld=lld -Wl,--build-id -o %t %s %{cflags}
// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
// RUN: llvm-readelf --notes %t && llvm-profdata show --binary-ids %t.profraw > FileCheck
Copy link
Contributor

Choose a reason for hiding this comment

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

These are now just writing to a file called FileCheck instead of invoking the FileCheck command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to get FileCheck to check the output of these commands (stdout). But maybe you're right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried updating line 12 to be:

RUN: llvm-readelf --notes %t > %t2 && llvm-profdata show --binary-ids %t.pr\ofraw >> %t2 | FileCheck %t2

When I do that and try running the test I get an error:

error: no check strings found with prefix 'CHECK:'

:-(

Copy link
Member

Choose a reason for hiding this comment

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

Can this lit internal shell still do grouping? It would need something like (llvm-readelf ... && llvm-profdata ...) | FileCheck %s for the current CHECK lines to work. If not, you could split them into separate --check-prefix commands.

Copy link
Member

Choose a reason for hiding this comment

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

RUN: llvm-readelf --notes %t > %t2 && llvm-profdata show --binary-ids %t.pr\ofraw >> %t2 | FileCheck %t2

Try ... && FileCheck %s <%t2? It needs the current file (%s) for the CHECK lines, and the test output on stdin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can this lit internal shell still do grouping?

It doesn't support subshells. The way we have been handling this is to have multiple commands write out to a file and then pass that to FileCheck using --input-file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RUN: llvm-readelf --notes %t > %t2 && llvm-profdata show --binary-ids %t.pr\ofraw >> %t2 | FileCheck %t2

Try ... && FileCheck %s <%t2? It needs the current file (%s) for the CHECK lines, and the test output on stdin.

This seems to work! Thank you!

@cmtice cmtice removed request for cuviper and fmayer September 10, 2025 21:12
@cmtice cmtice merged commit bc755ae into llvm:main Sep 11, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler-rt PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants