Skip to content

Add regression test for pclmulqdq inlining across target feature#157791

Open
kohsine wants to merge 1 commit into
rust-lang:mainfrom
kohsine:pclmulqdq-inlining
Open

Add regression test for pclmulqdq inlining across target feature#157791
kohsine wants to merge 1 commit into
rust-lang:mainfrom
kohsine:pclmulqdq-inlining

Conversation

@kohsine

@kohsine kohsine commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Regression test for #139029.

Checks that pclmulqdq intrinsics inline across target_feature.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2026
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2026
@rustbot

rustbot commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

r? @chenyukang

rustbot has assigned @chenyukang.
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 73 candidates
  • Random selection from 20 candidates

@folkertdev folkertdev 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.

r? me

It would be better to use minicore for this test, so it runs regardless of the host. There are plenty of examples of this, these tests use

//@ add-minicore
//@ compile-flags: -Copt-level=3 --target x86_64-unknown-linux-gnu
//@ needs-llvm-components: x86
#![feature(no_core)]
#![no_core]

extern crate minicore;
use minicore::*;

View changes since this review

@rustbot rustbot assigned folkertdev and unassigned chenyukang Jun 11, 2026
@folkertdev

Copy link
Copy Markdown
Contributor

Ah sorry, this wants to test core::arch really, then this is fine.

Comment on lines +12 to +14
// CHECK-LABEL: @reduce128_caller
// CHECK: call <2 x i64> @llvm.x86.pclmulqdq
// CHECK: call <2 x i64> @llvm.x86.pclmulqdq

@folkertdev folkertdev Jun 12, 2026

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.

Currently this only tests that

  • the reduce128_caller function exists
  • call <2 x i64> @llvm.x86.pclmulqdq occurs twice.

But this doesn't test that reduce128 actually got inlined.

View changes since the review

reduce128(a, b, keys)
}

unsafe fn reduce128(a: arch::__m128i, b: arch::__m128i, keys: arch::__m128i) -> arch::__m128i {

@folkertdev folkertdev Jun 12, 2026

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.

Suggested change
unsafe fn reduce128(a: arch::__m128i, b: arch::__m128i, keys: arch::__m128i) -> arch::__m128i {
// CHECK-LABEL: @reduce128
// CHECK: call <2 x i64> @llvm.x86.pclmulqdq
// CHECK: call <2 x i64> @llvm.x86.pclmulqdq
#[no_mangle]
unsafe fn reduce128(a: arch::__m128i, b: arch::__m128i, keys: arch::__m128i) -> arch::__m128i {

With these additional checks the call <2 x i64> @llvm.x86.pclmulqdq must occur 4 times in total, proving that inlining occurred.

View changes since the review


//! Regression test for https://github.com/rust-lang/rust/issues/139029
//!
//! pclmulqdq intrinsics should inline across target_feature

@folkertdev folkertdev Jun 12, 2026

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.

Suggested change
//! pclmulqdq intrinsics should inline across target_feature
//! The pclmulqdq intrinsics should inline into functions with the required target features.
*[View changes since the review](https://triagebot.infra.rust-lang.org/gh-changes-since/rust-lang/rust/157791/d2f24127d97d8c795d1cf5fbf6037f9885f34d39..e497947ec34512d57dd5fb3a0e24871c7c97337e)*

@rustbot rustbot 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 Jun 12, 2026
@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

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

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

4 participants