Skip to content
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

[profdata][nfc] Disable several tests on Windows #83907

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

alanzhao1
Copy link
Contributor

@alanzhao1 alanzhao1 commented Mar 4, 2024

Several profdata tests pass the byte 012 to printf. This causes these tests to fail when using GnuWin32's version of printf because printf will detect that 012 is the LF character and will prepend the byte 015 (CR) in front of LF.

This change is required after #82711 which bumped the version number.

Several profdata tests pass the byte 012 to printf. This causes these
tests to fail when using GnuWin32's version of printf because printf
will detect that 012 is the LF character and will prepend the byte 015
(CR) in front of LF.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Mar 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 4, 2024

@llvm/pr-subscribers-pgo

Author: Alan Zhao (alanzhao1)

Changes

Several profdata tests pass the byte 012 to printf. This causes these tests to fail when using GnuWin32's version of printf because printf will detect that 012 is the LF character and will prepend the byte 015 (CR) in front of LF.


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

11 Files Affected:

  • (modified) llvm/test/tools/llvm-profdata/binary-ids-padding.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/large-binary-id-size.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/raw-32-bits-be.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/raw-32-bits-le.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/raw-64-bits-be.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/raw-64-bits-le.test (+3)
  • (modified) llvm/test/tools/llvm-profdata/raw-two-profiles.test (+3)
diff --git a/llvm/test/tools/llvm-profdata/binary-ids-padding.test b/llvm/test/tools/llvm-profdata/binary-ids-padding.test
index 61881b69cfd5c0..292c582b45c52d 100644
--- a/llvm/test/tools/llvm-profdata/binary-ids-padding.test
+++ b/llvm/test/tools/llvm-profdata/binary-ids-padding.test
@@ -14,6 +14,9 @@
 // INSTR_PROF_RAW_HEADER(uint64_t, NumVTables, NumVTables)
 // INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
 
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 // There will be 2 20-byte binary IDs, so the total Binary IDs size will be 64 bytes.
diff --git a/llvm/test/tools/llvm-profdata/large-binary-id-size.test b/llvm/test/tools/llvm-profdata/large-binary-id-size.test
index 316a9a4c9df4ce..b62bdad4ddb219 100644
--- a/llvm/test/tools/llvm-profdata/large-binary-id-size.test
+++ b/llvm/test/tools/llvm-profdata/large-binary-id-size.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\40\0\0\0\0\0\0\0' >> %t.profraw
diff --git a/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test b/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test
index 8b686d5c50cb74..705e5efaf58759 100644
--- a/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test
+++ b/llvm/test/tools/llvm-profdata/malformed-not-space-for-another-header.test
@@ -14,6 +14,9 @@
 // INSTR_PROF_RAW_HEADER(uint64_t, NumVTables, NumVTables)
 // INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
 
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
diff --git a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
index 089afad4206223..157c13b926a7ed 100644
--- a/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
+++ b/llvm/test/tools/llvm-profdata/malformed-num-counters-zero.test
@@ -14,6 +14,9 @@
 // INSTR_PROF_RAW_HEADER(uint64_t, NumVTables, NumVTables)
 // INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
 
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
diff --git a/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test b/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
index e404ba4210cc14..83cf76f68fb635 100644
--- a/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
+++ b/llvm/test/tools/llvm-profdata/malformed-ptr-to-counter-array.test
@@ -14,6 +14,9 @@
 // INSTR_PROF_RAW_HEADER(uint64_t, NumVTables, NumVTables)
 // INSTR_PROF_RAW_HEADER(uint64_t, ValueKindLast, IPVK_Last)
 
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t.profraw
diff --git a/llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test b/llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test
index ee54bfb9785678..0f20a1b0b369c2 100644
--- a/llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test
+++ b/llvm/test/tools/llvm-profdata/misaligned-binary-ids-size.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t.profraw
 // We should fail on this because the binary IDs is not a multiple of 8 bytes.
diff --git a/llvm/test/tools/llvm-profdata/raw-32-bits-be.test b/llvm/test/tools/llvm-profdata/raw-32-bits-be.test
index 63782c8b94d4a5..fbd73ae30a5b33 100644
--- a/llvm/test/tools/llvm-profdata/raw-32-bits-be.test
+++ b/llvm/test/tools/llvm-profdata/raw-32-bits-be.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 // Header
 RUN: printf '\377lprofR\201' > %t
 RUN: printf '\0\0\0\0\0\0\0\12' >> %t
diff --git a/llvm/test/tools/llvm-profdata/raw-32-bits-le.test b/llvm/test/tools/llvm-profdata/raw-32-bits-le.test
index e9569bec1178bd..91f10fc8745867 100644
--- a/llvm/test/tools/llvm-profdata/raw-32-bits-le.test
+++ b/llvm/test/tools/llvm-profdata/raw-32-bits-le.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201Rforpl\377' > %t
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
diff --git a/llvm/test/tools/llvm-profdata/raw-64-bits-be.test b/llvm/test/tools/llvm-profdata/raw-64-bits-be.test
index 0bc579eec58abb..e7694a1b5a73b7 100644
--- a/llvm/test/tools/llvm-profdata/raw-64-bits-be.test
+++ b/llvm/test/tools/llvm-profdata/raw-64-bits-be.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\377lprofr\201' > %t
 RUN: printf '\0\0\0\0\0\0\0\12' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
diff --git a/llvm/test/tools/llvm-profdata/raw-64-bits-le.test b/llvm/test/tools/llvm-profdata/raw-64-bits-le.test
index ca9ea54c3f0146..99d486b8a51769 100644
--- a/llvm/test/tools/llvm-profdata/raw-64-bits-le.test
+++ b/llvm/test/tools/llvm-profdata/raw-64-bits-le.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t
diff --git a/llvm/test/tools/llvm-profdata/raw-two-profiles.test b/llvm/test/tools/llvm-profdata/raw-two-profiles.test
index 70a4210dea9f84..47cc6fa4fd7fe3 100644
--- a/llvm/test/tools/llvm-profdata/raw-two-profiles.test
+++ b/llvm/test/tools/llvm-profdata/raw-two-profiles.test
@@ -1,3 +1,6 @@
+// gnuwin32 printf does not work for this test because it will print \15 (CR)
+// whenever \12 (LF) is in the input string.
+UNSUPPORTED: system-windows
 RUN: printf '\201rforpl\377' > %t-foo.profraw
 RUN: printf '\12\0\0\0\0\0\0\0' >> %t-foo.profraw
 RUN: printf '\0\0\0\0\0\0\0\0' >> %t-foo.profraw

@alanzhao1 alanzhao1 changed the title [TypeProf][InstrPGO][nfc] Disable several tests on Windows [profdata][nfc] Disable several tests on Windows Mar 4, 2024
Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

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

lgtm

I'd suggest mentioning in the commit message that this is needed after 16e74fd which bumped the version number.

@alanzhao1
Copy link
Contributor Author

lgtm

I'd suggest mentioning in the commit message that this is needed after 16e74fd which bumped the version number.

Done

@alanzhao1 alanzhao1 closed this Mar 4, 2024
@alanzhao1 alanzhao1 reopened this Mar 4, 2024
@alanzhao1 alanzhao1 merged commit 922a431 into llvm:main Mar 4, 2024
9 of 10 checks passed
@alanzhao1 alanzhao1 deleted the bugfix/disable-windows-printf-tests branch March 4, 2024 21:16
@minglotus-6
Copy link
Contributor

thanks for figuring out why the test failed and making this change!

I did monitor the post-commit CI and the bots passed (from the green checkmark to the upper left of the commit).

For my information, where do you find the failures for GnuWin32's machine? And did you get a real Windows machine to debug the test failures?

@alanzhao1
Copy link
Contributor Author

This was detected while attempting to update clang for Chrome: https://g-issues.chromium.org/issues/327446719

I was able to repro this locally on a Windows machine using Chrome's package.py script, which uses GnuWin32.

Note that git bash's printf doesn't suffer from this issue, so you'll need to have lit explicitly use GnuWin32's printf.

@vvereschaka
Copy link
Contributor

These changes have broken few builders with the following error in the CMake step

-- Compiling and running to test HAVE_PTHREAD_AFFINITY
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/build/CMakeFiles/CMakeTmp/CMakeLists.txt:19 (add_executable):
  Cannot find source file:
    C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/cmake/pthread_affinity.cpp
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/build/CMakeFiles/CMakeTmp/CMakeLists.txt:19 (add_executable):
  No SOURCES given to target: cmTC_8cb19
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/cmake/CXXFeatureCheck.cmake:57 (try_run):
  Failed to generate test project build system.
Call Stack (most recent call first):
  C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/CMakeLists.txt:326 (cxx_feature_check)

would you take care of it?

@alanzhao1
Copy link
Contributor Author

These changes have broken few builders with the following error in the CMake step

-- Compiling and running to test HAVE_PTHREAD_AFFINITY
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/build/CMakeFiles/CMakeTmp/CMakeLists.txt:19 (add_executable):
  Cannot find source file:
    C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/cmake/pthread_affinity.cpp
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/build/CMakeFiles/CMakeTmp/CMakeLists.txt:19 (add_executable):
  No SOURCES given to target: cmTC_8cb19
CMake Error at C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/cmake/CXXFeatureCheck.cmake:57 (try_run):
  Failed to generate test project build system.
Call Stack (most recent call first):
  C:/buildbot/as-builder-3/llvm-clang-x86_64-win-fast/llvm-project/third-party/benchmark/CMakeLists.txt:326 (cxx_feature_check)

would you take care of it?

The buildbot blamed the wrong change. The breakage is actually due to #83488, which was reverted in aec6a04

@minglotus-6
Copy link
Contributor

This was detected while attempting to update clang for Chrome: https://g-issues.chromium.org/issues/327446719

I was able to repro this locally on a Windows machine using Chrome's package.py script, which uses GnuWin32.

Note that git bash's printf doesn't suffer from this issue, so you'll need to have lit explicitly use GnuWin32's printf.

Ack! The issue is configuration-sensitive. Thanks again for fixing this!

@vvereschaka
Copy link
Contributor

The buildbot blamed the wrong change. The breakage is actually due to #83488, which was reverted in aec6a04

got it! Sorry for the false alarm.

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.

None yet

5 participants