Skip to content

Updated slice tests to pass for big endian hosts for multiple-option-or-permutations.rs#151780

Merged
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
rwardd:ryan/fix-option-permutations
Mar 2, 2026
Merged

Updated slice tests to pass for big endian hosts for multiple-option-or-permutations.rs#151780
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
rwardd:ryan/fix-option-permutations

Conversation

@rwardd
Copy link
Contributor

@rwardd rwardd commented Jan 28, 2026

It was discovered that the FileCheck tests when performing an Option::or operation on a slice was failing when tested on a big endian host.

The compiler explorer link is here outlining the codegen output differences - https://rust.godbolt.org/z/qdE7d3G4f

This MR relaxes the constraints for the *slice_u8 variants of the test (by changing CHECK-NEXT to CHECK-DAG), whilst still maintaining the check for the necessary or logic.

Huge thanks to @Gelbpunkt for identifying this issue! It has been confirmed that this fix passes on a big endian target now as well.

Closes #151718

@rustbot rustbot added 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 Jan 28, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 28, 2026

r? @jieyouxu

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

@rwardd
Copy link
Contributor Author

rwardd commented Jan 28, 2026

r? @scottmcm

Was hoping to get some feedback from yourself, hopefully this one is still semi-fresh in memory :)
(Original PR - #150564)

@rustbot rustbot assigned scottmcm and unassigned jieyouxu Jan 28, 2026
// CHECK-NEXT: [[R:%.+]] = select i1 [[SOME_A]], i16 %0, i16 %1
// CHECK: ret i16 [[R]]
// CHECK-DAG: [[SOME_A:%.+]] = trunc i16 {{.*}} to i1
// CHECK-DAG: select i1 [[SOME_A]], i16 %0, i16 %1
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the use of CHECK-DAG here makes sense, as these instructions can only occur in this order (one depends on the other). Possibly you meant just CHECK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in retrospect CHECK-DAG isn't good here, I will refactor to use revisions and subsequently use LITTLE-NEXT, BIG-X.

// CHECK: ret i16 [[R]]
// CHECK-DAG: [[SOME_A:%.+]] = trunc i16 {{.*}} to i1
// CHECK-DAG: select i1 [[SOME_A]], i16 %0, i16 %1
// CHECK: ret i16 {{.*}}
Copy link
Member

Choose a reason for hiding this comment

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

This change to not say what's being returned feels like it's just making it a worse test. Notably, sure it's passing, but you needed to do this because there's two selects in the BE version, and failing if the LE version started doing that is good if you ask me.

I think it'd be better to use revisions to make these LITTLE-NEXT and add separate BIG-WHATEVER checks for the big-endian versions if they're this different.

Copy link
Contributor Author

@rwardd rwardd Jan 28, 2026

Choose a reason for hiding this comment

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

Agreed, when writing this I wasn't too happy that it wasn't exhaustively checking the BE output, whilst simultaneously making the LE test worse.

I'm currently working through using revisions, my current iteration looking like the following, using only-endian-big and ignore-endian-big to select the desired revision:

//@ revisions: LITTLE BIG
//@ [BIG] only-endian-big
//@ [LITTLE] ignore-endian-big

The only problem is that the only-endian-big doesn't exist in directive_names.rs, but it's still a valid directive according to the test directive docs.

Would it be appropriate to add only-endian-big to directive_names.rs in this PR?

@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 Jan 28, 2026
@rustbot
Copy link
Collaborator

rustbot commented Jan 28, 2026

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

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Feb 8, 2026
@rwardd rwardd force-pushed the ryan/fix-option-permutations branch from 61d5d8e to 5265ee0 Compare February 8, 2026 05:06
@rustbot
Copy link
Collaborator

rustbot commented Feb 8, 2026

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 8, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 8, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits PR has merge commits, merge with caution. labels Feb 8, 2026
@rwardd
Copy link
Contributor Author

rwardd commented Feb 8, 2026

@rustbot ready

Apologies for the delay in fixing this, I have added only-endian-big to directive_names.rs to allow for conditional BIG and LITTLE revisions.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2026
@Gelbpunkt
Copy link
Contributor

Heya, just chiming in again to confirm that the updated version still passes tests on big endian hosts :)

@Gelbpunkt
Copy link
Contributor

Gentle ping, the testsuite is still broken on big endian targets without this patch

@nikic
Copy link
Contributor

nikic commented Mar 2, 2026

Filed llvm/llvm-project#184131 for the missing optimization on big endian.

@nikic
Copy link
Contributor

nikic commented Mar 2, 2026

@bors r+

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 2, 2026

📌 Commit 5265ee0 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 Mar 2, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 2, 2026
…s, r=nikic

Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`

It was discovered that the FileCheck tests when performing an `Option::or` operation on a slice was failing when tested on a big endian host.

The compiler explorer link is here outlining the codegen output differences - https://rust.godbolt.org/z/qdE7d3G4f

This MR relaxes the constraints for the `*slice_u8` variants of the test (by changing `CHECK-NEXT` to `CHECK-DAG`), whilst still maintaining the check for the necessary `or` logic.

Huge thanks to @Gelbpunkt for identifying this issue! It has been confirmed that this fix passes on a big endian target now as well.

Closes rust-lang#151718
rust-bors bot pushed a commit that referenced this pull request Mar 2, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #153153 (add tests for thumb interworking)
 - #149328 (Add `String<A>` type with custom allocator parameter)
 - #151780 (Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`)
 - #153015 (core: make atomic primitives type aliases of `Atomic<T>`)
 - #153292 (tests: codegen-llvm: vec-calloc: do not require the uwtable attribute)

Failed merges:

 - #151864 (Implement AST -> HIR generics propagation in delegation)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 2, 2026
…s, r=nikic

Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`

It was discovered that the FileCheck tests when performing an `Option::or` operation on a slice was failing when tested on a big endian host.

The compiler explorer link is here outlining the codegen output differences - https://rust.godbolt.org/z/qdE7d3G4f

This MR relaxes the constraints for the `*slice_u8` variants of the test (by changing `CHECK-NEXT` to `CHECK-DAG`), whilst still maintaining the check for the necessary `or` logic.

Huge thanks to @Gelbpunkt for identifying this issue! It has been confirmed that this fix passes on a big endian target now as well.

Closes rust-lang#151718
rust-bors bot pushed a commit that referenced this pull request Mar 2, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #153153 (add tests for thumb interworking)
 - #149328 (Add `String<A>` type with custom allocator parameter)
 - #151780 (Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`)
 - #151962 (Fix next-solver ICE on PointeeSized goals)
 - #153015 (core: make atomic primitives type aliases of `Atomic<T>`)
 - #153161 (Rejig `rustc_with_all_queries!`)
 - #153191 (  don't emit `unused_results` lint for tuples of "trivial" types )
 - #153273 (vec/mod.rs: add missing period in "ie." in docs)
 - #153292 (tests: codegen-llvm: vec-calloc: do not require the uwtable attribute)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 2, 2026
…s, r=nikic

Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`

It was discovered that the FileCheck tests when performing an `Option::or` operation on a slice was failing when tested on a big endian host.

The compiler explorer link is here outlining the codegen output differences - https://rust.godbolt.org/z/qdE7d3G4f

This MR relaxes the constraints for the `*slice_u8` variants of the test (by changing `CHECK-NEXT` to `CHECK-DAG`), whilst still maintaining the check for the necessary `or` logic.

Huge thanks to @Gelbpunkt for identifying this issue! It has been confirmed that this fix passes on a big endian target now as well.

Closes rust-lang#151718
rust-bors bot pushed a commit that referenced this pull request Mar 2, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #153153 (add tests for thumb interworking)
 - #149328 (Add `String<A>` type with custom allocator parameter)
 - #151780 (Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`)
 - #151962 (Fix next-solver ICE on PointeeSized goals)
 - #153015 (core: make atomic primitives type aliases of `Atomic<T>`)
 - #153161 (Rejig `rustc_with_all_queries!`)
 - #153191 (  don't emit `unused_results` lint for tuples of "trivial" types )
 - #153273 (vec/mod.rs: add missing period in "ie." in docs)
 - #153292 (tests: codegen-llvm: vec-calloc: do not require the uwtable attribute)
 - #153293 (library: std: process: skip tests on Hermit)
 - #153301 (Do not ping kobzol on rustc-dev-guide changes)
rust-bors bot pushed a commit that referenced this pull request Mar 2, 2026
…uwer

Rollup of 10 pull requests

Successful merges:

 - #153153 (add tests for thumb interworking)
 - #151780 (Updated slice tests to pass for big endian hosts for `multiple-option-or-permutations.rs`)
 - #151962 (Fix next-solver ICE on PointeeSized goals)
 - #153015 (core: make atomic primitives type aliases of `Atomic<T>`)
 - #153161 (Rejig `rustc_with_all_queries!`)
 - #153191 (  don't emit `unused_results` lint for tuples of "trivial" types )
 - #153273 (vec/mod.rs: add missing period in "ie." in docs)
 - #153292 (tests: codegen-llvm: vec-calloc: do not require the uwtable attribute)
 - #153293 (library: std: process: skip tests on Hermit)
 - #153301 (Do not ping kobzol on rustc-dev-guide changes)
@rust-bors rust-bors bot merged commit 1654b17 into rust-lang:main Mar 2, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 2, 2026
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.

tests/codegen-llvm/issues/multiple-option-or-permutations.rs fails on big endian targets

6 participants