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

LLVM 18 issue: tests/ui/asm/inline-syntax.rs#arm broken after LLVM https://github.com/llvm/llvm-project/commit/96aca7c51701f9b3c5dd8567fcddf29492008e6d #119120

Closed
krasimirgg opened this issue Dec 19, 2023 · 5 comments · Fixed by #120094
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@krasimirgg
Copy link
Contributor

krasimirgg commented Dec 19, 2023

After llvm/llvm-project@96aca7c, rustc starts emitting duplicate diagnostics while running the tests/ui/asm/inline-syntax.rs#arm test:
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/24569#018c7e16-58de-4bfd-a579-6b4a83bec58d/244-1054.

Looking at the commit description, the change adapts the behavior of emitting these diagnostics to be consistent across different linking modes, but doesn't prevent the possibility for duplicated diagnostics. Seems like a quick fix would be to add -Zdeduplicate-diagnostics to that test.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 19, 2023
@krasimirgg
Copy link
Contributor Author

@rustbot label: +A-LLVM

@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Dec 19, 2023
@krasimirgg
Copy link
Contributor Author

@rustbot label: -needs-triage

@rustbot rustbot removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 19, 2023
@Noratrieb Noratrieb added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Dec 20, 2023
@nikic
Copy link
Contributor

nikic commented Dec 21, 2023

Is the duplicate diagnostic an LLVM bug?

@krasimirgg
Copy link
Contributor Author

krasimirgg commented Dec 21, 2023

There are two effects at play:

  1. duplicated diagnostic
  2. extra note for the assembly syntax is emitted

I'm not sure if the duplicated diagnostic is an LLVM bug. It's however consistent before and after this change: the first diagnostic is duplicated in both cases.

The test failure is due to the second effect: after this change, there's an extra note emitted now for the (duplicated) diagnostic. This patch https://github.com/rust-lang/rust/pull/119185/files illustrates the difference and is sufficient to get the test passing with LLVM at HEAD (but obviously doesn't work for older versions).

@nikic
Copy link
Contributor

nikic commented Dec 21, 2023

Ah, I see. In that case we should probably just copy the test and have one <18 and one >= 18 version.

@bors bors closed this as completed in 135476b Jan 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 19, 2024
Rollup merge of rust-lang#120094 - krasimirgg:inline-asm-llvm-18, r=nikic

tests/ui/asm/inline-syntax: adapt for LLVM 18

Fixes rust-lang#119120.
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. A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants