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

tests/assembly/x86_64-bigint-add.rs broken with LLVM 20 #133754

Closed
krasimirgg opened this issue Dec 2, 2024 · 2 comments · Fixed by #135764
Closed

tests/assembly/x86_64-bigint-add.rs broken with LLVM 20 #133754

krasimirgg opened this issue Dec 2, 2024 · 2 comments · Fixed by #135764
Assignees
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 2, 2024

It looks like tests/assembly/x86_64-bigint-add.rs broken, newly added in 9836196, produces slightly different assembly with LLVM-at-HEAD:

https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/33017#01938775-555b-4915-9c10-08f40d14cdad/945-946

@scottmcm: does the new assembly produced look good? Happy to adapt the test for this.

cc: @scottmcm @durin42

@krasimirgg krasimirgg added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Dec 2, 2024
@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 2, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-testsuite Area: The testsuite used to check the correctness of rustc and removed C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Dec 2, 2024
@nikic nikic self-assigned this Jan 20, 2025
@nikic
Copy link
Contributor

nikic commented Jan 20, 2025

It looks like this is just a difference in unroll factor: https://www.diffchecker.com/qUdwbutN/ LLVM 20 unrolls this loop less aggressively.

jhpratt added a commit to jhpratt/rust that referenced this issue Jan 23, 2025
…wiser

Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang#132957.
Fixes rust-lang#133754.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
…wiser

Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang#132957.
Fixes rust-lang#133754.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
…wiser

Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang#132957.
Fixes rust-lang#133754.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 25, 2025
…wiser

Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang#132957.
Fixes rust-lang#133754.
jhpratt added a commit to jhpratt/rust that referenced this issue Jan 26, 2025
…wiser

Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang#132957.
Fixes rust-lang#133754.
@bors bors closed this as completed in 01a26c0 Jan 26, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this issue Jan 27, 2025
Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang/rust#132957.
Fixes rust-lang/rust#133754.
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jan 28, 2025
Fix tests on LLVM 20

For sparcv8plus.rs, duplicate the test for LLVM 19 and LLVM 20. LLVM 20 resolves one of the FIXME in the test.

For x86_64-bigint-add.rs split the check lines for LLVM 19 and LLVM 20. The difference in codegen here is due to a difference in unroll factor, which I believe is not what the test is interested in.

Fixes rust-lang/rust#132957.
Fixes rust-lang/rust#133754.
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