Skip to content

Don't ignore unknown LLVM diagnostics - #164578

Merged
rust-bors[bot] merged 5 commits into
rust-lang:mainfrom
nikic:llvm-diags
Nov 1, 2026
Merged

rust-bors[bot] merged 5 commits into
rust-lang:mainfrom
nikic:llvm-diags

Conversation

@nikic

@nikic nikic commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

View all comments

Rust currently ignores LLVM diagnostic kinds that it does not know about. This means that we sometimes ignore error diagnostics, which can then result in silent miscompiles.

This exposes LLVMGetDiagInfoSeverity returning DiagnosticSeverity, renamed from DiagnosticLevel to match LLVM terminology. Custom code to fetch the diagnostic severity for inline asm diagnostics can then be dropped.

For unknown diagnostics, we emit a warning/error depending on severity. Notes and remarks continue to be ignored.

One oddity I ran into along the way is that the linker diagnostic has severity error, but we only report a warning. I've kept this for now as we still error down the line and this ends up providing a bit more information (which file the duplicate symbol originates from).

Fixes #146393.

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 19, 2026
@rustbot

rustbot commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

r? @mati865

rustbot has assigned @mati865.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 77 candidates
  • Random selection from 18 candidates

@rust-log-analyzer

This comment has been minimized.

@mati865 mati865 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I went through the rabbit hole and the warning for PGO was added as the initial PGO support 8a4cebd (#48346) with the reasoning that should be at least a warning.
Then, #52474 built on top of it adding warnings for previously ignored LTO errors.

I've kept this for now as we still error down the line and this ends up providing a bit more information (which file the duplicate symbol originates from).

I agree that seeing where the error comes from is potentially useful for non-mangled symbols, but currently this seems to depend entirely on the next stage to emit an error upon encountering bad inputs. I imagine (uneducated guess) there might be cases where we print LLVM's error as a warning, but the next stage emits just a warning or is silent.

r=me with sentence above and comments addressed or discarded.

View changes since this review

dcx.emit_warn(FromLlvmDiag { message });
}
llvm::diagnostic::Unsupported(diagnostic_ref) => {
llvm::diagnostic::PGO(diagnostic_ref)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding PGO test akin to lto-duplicate-symbol shouldn't be hard, no? Could be a good E-easy issue.

llvm::diagnostic::Unsupported(diagnostic_ref) => {
llvm::diagnostic::PGO(diagnostic_ref)
| llvm::diagnostic::Unsupported(diagnostic_ref)
| llvm::diagnostic::UnknownDiagnostic(diagnostic_ref) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just asking because I'm not familiar with how we want to handle LLVM errors.
Would it be useful to split UnknownDiagnostic variant into LLVM's unknown errors and "LLVM errors that we didn't map yet", so we can indicate the not mapped ones here?
Acting only when we hit a not yet mapped error that needs special handling seems fine too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TBH I think we should rather go the other way around and remove any diagnostic kinds that we don't specially handle, i.e. drop PGO and Unsupported and use UnknownDiagnostic for everything. Apart from a few special cases (inline asm and optimization remarks), Rust mostly shouldn't care about where the diagnostic originates and just forward it.

@nikic

nikic commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

I've pushed two additional changes here. One to remove the Diagnostic variants that don't get special treatment (they're all Diagnostic::Other now), and one to remove the special Linker handling as well. Overall, I think that special case isn't worth it, esp. with the risk that we may end up not erroring after a downgraded warning.

@mati865

mati865 commented Oct 20, 2026

Copy link
Copy Markdown
Member

Sounds good, thanks!

@bors r+

@rust-bors

rust-bors Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

📌 Commit b95ad06 has been approved by mati865

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Oct 20, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 20, 2026
Don't ignore unknown LLVM diagnostics

Rust currently ignores LLVM diagnostic kinds that it does not know about. This means that we sometimes ignore error diagnostics, which can then result in silent miscompiles.

This exposes `LLVMGetDiagInfoSeverity` returning `DiagnosticSeverity`, renamed from `DiagnosticLevel` to match LLVM terminology. Custom code to fetch the diagnostic severity for inline asm diagnostics can then be dropped.

For unknown diagnostics, we emit a warning/error depending on severity. Notes and remarks continue to be ignored.

One oddity I ran into along the way is that the linker diagnostic has severity error, but we only report a warning. I've kept this for now as we still error down the line and this ends up providing a bit more information (which file the duplicate symbol originates from).

Fixes rust-lang#146393.
rust-bors Bot pushed a commit that referenced this pull request Oct 20, 2026
…uwer

Rollup of 6 pull requests

Successful merges:

 - #163104 ([debuginfo] Emit `usize` and `isize` as typedefs)
 - #164488 (Don't try to skip anything in `Skip::advance_by(0)`)
 - #164578 (Don't ignore unknown LLVM diagnostics)
 - #164611 (Improve bit permutation codegen)
 - #162668 (document that `!` does bitwise and logical negation)
 - #164264 (lint: suggest `fXX::is_nan` for unsuffixed NaN comparisons)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 20, 2026
Don't ignore unknown LLVM diagnostics

Rust currently ignores LLVM diagnostic kinds that it does not know about. This means that we sometimes ignore error diagnostics, which can then result in silent miscompiles.

This exposes `LLVMGetDiagInfoSeverity` returning `DiagnosticSeverity`, renamed from `DiagnosticLevel` to match LLVM terminology. Custom code to fetch the diagnostic severity for inline asm diagnostics can then be dropped.

For unknown diagnostics, we emit a warning/error depending on severity. Notes and remarks continue to be ignored.

One oddity I ran into along the way is that the linker diagnostic has severity error, but we only report a warning. I've kept this for now as we still error down the line and this ends up providing a bit more information (which file the duplicate symbol originates from).

Fixes rust-lang#146393.
rust-bors Bot pushed a commit that referenced this pull request Oct 20, 2026
…uwer

Rollup of 14 pull requests

Successful merges:

 - #163575 (core: Add examples and assembly tests for `abort_immediate`)
 - #163664 (Add new `RUSTC_BOOTSTRAP=-allow-unstable-compiler-flags` mode for testing stable compiler behavior in UI tests)
 - #163104 ([debuginfo] Emit `usize` and `isize` as typedefs)
 - #163338 (MIR move elimination [4/6]: MoveElimination)
 - #163701 (add ReadDir::dir to get the directory handle of the underlying directory)
 - #164488 (Don't try to skip anything in `Skip::advance_by(0)`)
 - #164578 (Don't ignore unknown LLVM diagnostics)
 - #164611 (Improve bit permutation codegen)
 - #164612 (std: Combine `is_powerpc64_feature_detected` into `is_powerpc_feature_detected`)
 - #162668 (document that `!` does bitwise and logical negation)
 - #164264 (lint: suggest `fXX::is_nan` for unsuffixed NaN comparisons)
 - #164509 (layout invariant: turn delayed bug back into proper assertion)
 - #164594 (rustdoc: Correctly handle inner `deprecated` attribute on modules)
 - #164629 (arch::breakpoint: avoid confusing 'equivalent' terminology)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 20, 2026
Don't ignore unknown LLVM diagnostics

Rust currently ignores LLVM diagnostic kinds that it does not know about. This means that we sometimes ignore error diagnostics, which can then result in silent miscompiles.

This exposes `LLVMGetDiagInfoSeverity` returning `DiagnosticSeverity`, renamed from `DiagnosticLevel` to match LLVM terminology. Custom code to fetch the diagnostic severity for inline asm diagnostics can then be dropped.

For unknown diagnostics, we emit a warning/error depending on severity. Notes and remarks continue to be ignored.

One oddity I ran into along the way is that the linker diagnostic has severity error, but we only report a warning. I've kept this for now as we still error down the line and this ends up providing a bit more information (which file the duplicate symbol originates from).

Fixes rust-lang#146393.
rust-bors Bot pushed a commit that referenced this pull request Oct 20, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #163664 (Add new `RUSTC_BOOTSTRAP=-allow-unstable-compiler-flags` mode for testing stable compiler behavior in UI tests)
 - #163104 ([debuginfo] Emit `usize` and `isize` as typedefs)
 - #163338 (MIR move elimination [4/6]: MoveElimination)
 - #163701 (add ReadDir::dir to get the directory handle of the underlying directory)
 - #164488 (Don't try to skip anything in `Skip::advance_by(0)`)
 - #164578 (Don't ignore unknown LLVM diagnostics)
 - #164611 (Improve bit permutation codegen)
 - #164612 (std: Combine `is_powerpc64_feature_detected` into `is_powerpc_feature_detected`)
 - #162668 (document that `!` does bitwise and logical negation)
 - #164264 (lint: suggest `fXX::is_nan` for unsuffixed NaN comparisons)
 - #164509 (layout invariant: turn delayed bug back into proper assertion)
 - #164594 (rustdoc: Correctly handle inner `deprecated` attribute on modules)
 - #164629 (arch::breakpoint: avoid confusing 'equivalent' terminology)
rust-bors Bot pushed a commit that referenced this pull request Oct 20, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #163664 (Add new `RUSTC_BOOTSTRAP=-allow-unstable-compiler-flags` mode for testing stable compiler behavior in UI tests)
 - #163104 ([debuginfo] Emit `usize` and `isize` as typedefs)
 - #163338 (MIR move elimination [4/6]: MoveElimination)
 - #163701 (add ReadDir::dir to get the directory handle of the underlying directory)
 - #164488 (Don't try to skip anything in `Skip::advance_by(0)`)
 - #164578 (Don't ignore unknown LLVM diagnostics)
 - #164611 (Improve bit permutation codegen)
 - #164612 (std: Combine `is_powerpc64_feature_detected` into `is_powerpc_feature_detected`)
 - #162668 (document that `!` does bitwise and logical negation)
 - #164264 (lint: suggest `fXX::is_nan` for unsuffixed NaN comparisons)
 - #164509 (layout invariant: turn delayed bug back into proper assertion)
 - #164594 (rustdoc: Correctly handle inner `deprecated` attribute on modules)
 - #164629 (arch::breakpoint: avoid confusing 'equivalent' terminology)
@JonathanBrouwer

Copy link
Copy Markdown
Member

💔 I suspect this PR failed tests as part of a rollup
@bors r-

After fixing the problem, consider running a try job for the failed job before re-approving.

Link to failure: #164634 (comment)

@bors try jobs=test-x86_64-gnu-nopt

@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 20, 2026
@rust-bors

rust-bors Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

This PR was contained in a rollup (#164634), which was unapproved.

View changes since this unapproval

@rust-bors

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 26, 2026
@rust-bors

rust-bors Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

💔 Test for 42df3ea failed: CI. Failed job:

nikic added 2 commits October 26, 2026 14:24
This preserves the existing behavior. Enabling these warnings is
tracked in a new issue.
Apparently this depends on where the tests are run from?
@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2026
@nikic

nikic commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Meh. Dropped the forced -C opt-level=0 for the unoptimized revision of huge-stacks.rs again, because that fails on aarch64-pc-windows-msvc. Failure tracked at #164988.

@bors try jobs=test-aarch64-msvc-1

rust-bors Bot pushed a commit that referenced this pull request Oct 26, 2026
Don't ignore unknown LLVM diagnostics


try-job: test-aarch64-msvc-1
@rust-bors

This comment has been minimized.

@rust-bors

rust-bors Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 0e10d56 (0e10d56fb6f1e4b2ad9d5ccfcfe310c29609f8fd)
Base parent: 019b59d (019b59d8101bb3e141f1b880684747b4217beeb6)

@saethlin

Copy link
Copy Markdown
Member

@nikic Are you looking for another approval? I've looked over the code and it seems good to me.

@nikic

nikic commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Let's see if fourth time's the charm...

@bors r=mati865

@rust-bors

rust-bors Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 4980e18 has been approved by mati865

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2026
@rust-bors

This comment has been minimized.

@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 1, 2026
@rust-bors

rust-bors Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: mati865
Duration: 2h 52m 22s
Pushing f90a2d2 to main...

@rust-bors
rust-bors Bot merged commit f90a2d2 into rust-lang:main Nov 1, 2026
16 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor
What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing eec3f1e (parent) -> f90a2d2 (this PR)

Test differences

Show 8 test diffs

Stage 1

  • [ui (polonius)] tests/ui/target-feature/too-many-reserved-regs.rs: [missing] -> pass (J0)
  • [ui] tests/ui/target-feature/too-many-reserved-regs.rs: [missing] -> pass (J3)

Stage 2

  • [ui] tests/ui/target-feature/too-many-reserved-regs.rs: [missing] -> ignore (backend gcc cannot build for target riscv64gc-unknown-linux-gnu) (J1)
  • [ui] tests/ui/target-feature/too-many-reserved-regs.rs: [missing] -> pass (J2)

Additionally, 4 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard f90a2d27c2a1e18f10f4fd84c0dcd02710c37de8 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. test-x86_64-gnu-gcc-core-tests: 16m 6s -> 1h 12m (+351.1%)
  2. dist-i586-gnu-i586-i686-musl: 22m 44s -> 1h 22m (+265.0%)
  3. dist-android: 31m 18s -> 1h 32m (+196.2%)
  4. test-x86_64-rust-for-linux: 40m 33s -> 1h 54m (+182.3%)
  5. dist-i686-msvc: 40m 26s -> 1h 48m (+167.1%)
  6. test-x86_64-gnu-llvm-22: 38m 20s -> 1h 35m (+149.4%)
  7. test-x86_64-gnu-llvm-22-1: 50m 12s -> 1h 58m (+135.7%)
  8. dist-i686-mingw: 49m 15s -> 1h 54m (+132.4%)
  9. test-pr-check-1: 41m 10s -> 1h 34m (+128.5%)
  10. dist-ohos-armv7: 55m 10s -> 1h 58m (+114.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f90a2d2): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
-0.2% [-0.2%, -0.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.2% [-0.2%, -0.2%] 1

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (primary -2.2%, secondary 2.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
7.7% [7.5%, 7.8%] 2
Improvements ✅
(primary)
-2.2% [-2.5%, -1.8%] 2
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 2
All ❌✅ (primary) -2.2% [-2.5%, -1.8%] 2

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 488.328s -> 486.771s (-0.32%)
Artifact size: 406.66 MiB -> 406.65 MiB (-0.00%)

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

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. merged-by-bors This PR was explicitly merged by bors. 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.

Incorrect codegen (instead of error) since 1.87.0, when all but one register is reserved on RISC-V.

7 participants