Skip to content

test carryless_mul codegen#157831

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
folkertdev:carryless-mul-test
Jun 16, 2026
Merged

test carryless_mul codegen#157831
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
folkertdev:carryless-mul-test

Conversation

@folkertdev

@folkertdev folkertdev commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

tracking issue: #152080

Test the codegen of carryless_mul with LLVM 23, which has custom lowerings for x86_64, aarch64 and riscv64.

You can also see that the default expansion (when there is no special intruction available) is quite large still...

So LLVM default over 3X the number of instructions for the 64-bit and 128-bit case.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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 Jun 12, 2026
@folkertdev

Copy link
Copy Markdown
Contributor Author

Is it OK to add tests that only test the next LLVM? Otherwise this can be put on ice for a couple of weeks.

r? nikic

@folkertdev folkertdev marked this pull request as ready for review June 12, 2026 21:09
@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/compiletest

cc @jieyouxu

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 12, 2026

@nikic nikic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it OK to add tests that only test the next LLVM? Otherwise this can be put on ice for a couple of weeks.

Yes, that's fine.

It's a bit unfortunate that this test depends on target availability. I guess making this a minicore test by directly calling the intrinsic is bad practice?

View changes since this review

Comment thread tests/assembly-llvm/carryless-mul.rs Outdated
fn carryless_mul_u64(a: u64, b: u64) -> u64 {
// CHECK-LABEL: carryless_mul_u64:
// x86_64: pclmulqdq
// aarch64: pmull v0.1q, v1.1d, v0.1d

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The register matching here looks fragile? Like this one uses v1.1d, v0.1d, while the others are v0.1d, v1.1d. That seems like something that can easily flip.

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.

Fixed

@folkertdev folkertdev force-pushed the carryless-mul-test branch from e22bc59 to 9b88d5e Compare June 15, 2026 19:39

@folkertdev folkertdev left a comment

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.

I guess making this a minicore test by directly calling the intrinsic is bad practice?

Not per se, but this setup also tests that we hook up the LLVM intrinsic correctly (and e.g. don't accidentally use the fallback body).

View changes since this review

Comment thread tests/assembly-llvm/carryless-mul.rs Outdated
fn carryless_mul_u64(a: u64, b: u64) -> u64 {
// CHECK-LABEL: carryless_mul_u64:
// x86_64: pclmulqdq
// aarch64: pmull v0.1q, v1.1d, v0.1d

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.

Fixed

@nikic

nikic commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@rust-bors

rust-bors Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 9b88d5e has been approved by nikic

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 Jun 15, 2026
rust-bors Bot pushed a commit that referenced this pull request Jun 16, 2026
Rollup of 6 pull requests

Successful merges:

 - #157707 (Introduce `-Z lint-rust-version`)
 - #157748 (rustc_public: make sure hidden fields have their accessors)
 - #157831 (test `carryless_mul` codegen)
 - #157879 (bootstrap: fix inverted success check in PowerShell download fallback)
 - #157933 (Use constant for detecting thin pointer formatting)
 - #157934 (update AttributeTemplate docs)
@rust-bors rust-bors Bot merged commit 28c31cc into rust-lang:main Jun 16, 2026
13 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 16, 2026
rust-timer added a commit that referenced this pull request Jun 16, 2026
Rollup merge of #157831 - folkertdev:carryless-mul-test, r=nikic

test `carryless_mul` codegen

tracking issue: #152080

Test the codegen of `carryless_mul` with LLVM 23, which has custom lowerings for x86_64, aarch64 and riscv64.

- Rust to LLVM IR: https://godbolt.org/z/sM914e4fo
- LLVM IR to assembly: https://godbolt.org/z/5Y7naa4cY

You can also see that the default expansion (when there is no special intruction available) is quite large still...

- manual https://godbolt.org/z/hbEe3WMdW (based on #152132 (comment))
- LLVM https://godbolt.org/z/577Wb9E99

So LLVM default over 3X the number of instructions for the 64-bit and 128-bit case.
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-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.

3 participants