Skip to content

Conversation

@Zalathar
Copy link
Member

@Zalathar Zalathar commented Dec 5, 2025

Tests for -Zdebuginfo-compression=zstd need to be skipped if LLVM was built without support for zstd compression.

Currently, compiletest relies on messy and fragile heuristics to detect whether the compiler's LLVM was built with zstd support. But the compiler itself already knows whether LLVM has zstd or not, so it's easier for compiletest to just ask the compiler.


Originally I was intending for this to be a --print=debuginfo-compression flag that would print out a list of values supported by -Zdebuginfo-compression=. I got that working locally, but it was more complex than I was happy with (in both rustc and compiletest), so I decided to cut scope and instead add a very narrow perma-unstable print request instead.

There is always a circularity hazard whenever we ask the compiler-under-test for information about how to test it. But in this case, the underlying compiler code is fairly simple, whereas the previous heuristics were inherently messy and unreliable anyway.

Tests for `-Zdebuginfo-compression=zstd` need to be skipped if LLVM was built
without support for zstd compression.

Currently, compiletest relies on messy and fragile heuristics to detect whether
the compiler's LLVM was built with zstd support. But the compiler itself
already knows whether LLVM has zstd or not, so it's easier for compiletest to
just ask the compiler.
@rustbot
Copy link
Collaborator

rustbot commented Dec 5, 2025

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 5, 2025

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

@jieyouxu
Copy link
Member

jieyouxu commented Dec 5, 2025

You can r=me once PR CI is green.
r? me

@rustbot rustbot assigned jieyouxu and unassigned wesleywiser Dec 5, 2025
@Zalathar
Copy link
Member Author

Zalathar commented Dec 5, 2025

@bors r=jieyouxu

@bors
Copy link
Collaborator

bors commented Dec 5, 2025

📌 Commit 84ff44c has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2025
bors added a commit that referenced this pull request Dec 5, 2025
Rollup of 11 pull requests

Successful merges:

 - #148662 (alloc: Document panics when allocations will exceed max)
 - #148811 (core docs: rewrite `panic::Location::caller` with visual line/column numbers)
 - #149101 (Improve mutable-binding suggestion to include name)
 - #149477 (float::maximum/minimum: make docs more streamlined)
 - #149547 (library: Rename `IterRange*` to `Range*Iter`)
 - #149548 (Generate delegation error body when delegation is not resolved)
 - #149630 (Check identifiers defined in macros when suggesting identifiers hidden by hygiene)
 - #149647 (Add regression test for 141845)
 - #149661 (Fix for LLVM22 making lowering decisions dependent on RuntimeLibraryInfo.)
 - #149666 (Add perma-unstable `--print=backend-has-zstd` for use by compiletest)
 - #149671 (interpret: test SNaN handling of float min/max and update comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8e46521 into rust-lang:main Dec 5, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Dec 5, 2025
rust-timer added a commit that referenced this pull request Dec 5, 2025
Rollup merge of #149666 - Zalathar:backend-has-zstd, r=jieyouxu

Add perma-unstable `--print=backend-has-zstd` for use by compiletest

Tests for `-Zdebuginfo-compression=zstd` need to be skipped if LLVM was built without support for zstd compression.

Currently, compiletest relies on messy and fragile heuristics to detect whether the compiler's LLVM was built with zstd support. But the compiler itself already knows whether LLVM has zstd or not, so it's easier for compiletest to just ask the compiler.

---

Originally I was intending for this to be a `--print=debuginfo-compression` flag that would print out a list of values supported by `-Zdebuginfo-compression=`. I got that working locally, but it was more complex than I was happy with (in both rustc and compiletest), so I decided to cut scope and instead add a very narrow perma-unstable print request instead.

There is always a circularity hazard whenever we ask the compiler-under-test for information about how to test it. But in this case, the underlying compiler code is fairly simple, whereas the previous heuristics were inherently messy and unreliable anyway.
@Zalathar Zalathar deleted the backend-has-zstd branch December 5, 2025 23:05
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 6, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#148662 (alloc: Document panics when allocations will exceed max)
 - rust-lang/rust#148811 (core docs: rewrite `panic::Location::caller` with visual line/column numbers)
 - rust-lang/rust#149101 (Improve mutable-binding suggestion to include name)
 - rust-lang/rust#149477 (float::maximum/minimum: make docs more streamlined)
 - rust-lang/rust#149547 (library: Rename `IterRange*` to `Range*Iter`)
 - rust-lang/rust#149548 (Generate delegation error body when delegation is not resolved)
 - rust-lang/rust#149630 (Check identifiers defined in macros when suggesting identifiers hidden by hygiene)
 - rust-lang/rust#149647 (Add regression test for 141845)
 - rust-lang/rust#149661 (Fix for LLVM22 making lowering decisions dependent on RuntimeLibraryInfo.)
 - rust-lang/rust#149666 (Add perma-unstable `--print=backend-has-zstd` for use by compiletest)
 - rust-lang/rust#149671 (interpret: test SNaN handling of float min/max and update comments)

r? `@ghost`
`@rustbot` modify labels: rollup
PrintKind::BackendHasZstd => {
let has_zstd = llvm::LLVMRustLLVMHasZstdCompression();
writeln!(out, "{has_zstd}").unwrap();
}
Copy link
Member

Choose a reason for hiding this comment

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

There is no default implementation of this print request, which causes compiletest to break for all non-llvm backends unless they explicitly add an implementation for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that's surprising, sorry.

I had intended to write this in a way that was friendly to other backends, but then I went back and changed things for other reasons and that property was lost.

Copy link
Member

Choose a reason for hiding this comment

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

Worked around this on the cg_clif side for now. cg_gcc would also need changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does the failure only occur when a non-LLVM backend is the default? Or does it break even when LLVM is the default and a different backend is selected?

Copy link
Member Author

Choose a reason for hiding this comment

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

My tentative fix is to add this to llvm_has_zstd in src/tools/compiletest/src/directives/needs.rs:

    if !config.default_codegen_backend.is_llvm() {
        return false;
    }

But I can't get any non-LLVM backends to work on my system at the moment, so I have no way to test this fix.

Copy link
Member

Choose a reason for hiding this comment

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

Given that you don't pass -Zcodegen-backend even when the user selects a different codegen backend, this should right now only break when a non-LLVM codegen backend is the default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that probably explains why it wasn't caught by CI.

I have a backend-agnostic fix posted at #149764.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants