Skip to content

[modules][test] Bump maximum size for embed-files-compressed.cpp for the sake of RISC-V #111360

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

Conversation

asb
Copy link
Contributor

@asb asb commented Oct 7, 2024

The size of a.pcm produced by an RV64 clang running natively is 67416 bytes, causing this test to fail. Bump up the maximum size. The test still retains its original intent as far as I can see, as it's really trying to ensure that compressing kicks in and reduces the multi-megabyte input.

…the sake of RISC-V

The size of a.pcm produced by an RV64 clang running natively is 67416
bytes, causing this test to fail. Bump up the maximum size. The test
still retains its original intent as far as I can see, as it's really
trying to ensure that compressing kicks in and reduces the
multi-megabyte input.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Alex Bradbury (asb)

Changes

The size of a.pcm produced by an RV64 clang running natively is 67416 bytes, causing this test to fail. Bump up the maximum size. The test still retains its original intent as far as I can see, as it's really trying to ensure that compressing kicks in and reduces the multi-megabyte input.


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

1 Files Affected:

  • (modified) clang/test/Modules/embed-files-compressed.cpp (+2-2)
diff --git a/clang/test/Modules/embed-files-compressed.cpp b/clang/test/Modules/embed-files-compressed.cpp
index 873b3082a2fdfa..aca9983ff160b6 100644
--- a/clang/test/Modules/embed-files-compressed.cpp
+++ b/clang/test/Modules/embed-files-compressed.cpp
@@ -17,7 +17,7 @@
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a -emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 60KB:
+// Check that the resulting file is under 80KB:
 //
 // RUN: wc -c %t/a.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-7][0-9][0-9][0-9][0-9]($|[^0-9])}}

@asb
Copy link
Contributor Author

asb commented Oct 7, 2024

In terms of why it's larger on RISC-V and has got bigger, I don't know enough about C++ modules to investigate much. -module-file-info wasn't very informative. The last commit that bumped up the maximum size was fa45f81 suggesting it may be increasing as new types are added to the RISC-V vector extension.

Copy link
Member

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

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

Thanks.

The underlying issue worths more investigating. But giving we don't have enough developers on modules and RV, maybe this is the best thing we can do now.

@asb asb merged commit 975da02 into llvm:main Oct 8, 2024
12 checks passed
asb added a commit to asb/llvm-project that referenced this pull request Jan 22, 2025
…RISC-V failure

I'm not sure why the test is larger for RISC-V than other targets, but
we saw this before with llvm#111360.

The file is just over the current 60KB limit:

```
62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm
```
asb added a commit that referenced this pull request Jan 23, 2025
…RISC-V failure (#123959)

I'm not sure why the test is larger for RISC-V than other targets, but
we saw this before with #111360.

The file is just over the current 60KB limit:

```
62772 /home/asb/llvm-project/build/stage2/tools/clang/test/Modules/Output/empty.modulemap.tmp/base.pcm
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants