tests: allow trunc/select instructions to be missing #130454

durin42 · 2024-09-17T00:49:28Z

On LLVM 20, these instructions already get eliminated, which at least partially satisfies a TODO. I'm not talented enough at using FileCheck to try and constrain this further, but if we really want to we could copy an LLVM 20 specific version of this test that would restore it to being CHECK-NEXT: insertvalue ...

@rustbot label: +llvm-main
r? @DianQK

rustbot · 2024-09-17T00:49:35Z

Failed to set assignee to DianQK: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

durin42 · 2024-09-17T00:53:24Z

r? cuviper

I guess

durin42 · 2024-09-17T00:54:10Z

oh, on second thought

r? @cjgillot

since they reviewed the other change here

tests/codegen/try_question_mark_nop.rs

workingjubilee · 2024-09-17T02:02:51Z

tests/codegen/try_question_mark_nop.rs

-    // CHECK-NEXT: insertvalue { i32, i32 } poison, i32 [[FIRST]]
+    // NINETEEN-NEXT: [[TRUNC:%.*]] = trunc nuw i32 %0 to i1
+    // NINETEEN-NEXT: [[FIRST:%.*]] = select i1 [[TRUNC]], i32 %0
+    // CHECK-NEXT: insertvalue { i32, i32 } poison, i32 %0, 0


one thing that might give us guff is this %0 here, since across revisions it might have a different name, but we can just drop the assertion on a specific var identity if that does indeed cause a problem, or use {{%[0-9]}} or whatever

Yeah, I thought about that, but figured the function is probably simple enough it won't ever be a problem.

yeah, if it passes now then it's fine since there won't be enough intermediate vars in later versions to ever even raise the question.

workingjubilee · 2024-09-17T04:15:55Z

and it does!
@bors r+ rollup

bors · 2024-09-17T04:15:58Z

📌 Commit 978f10d has been approved by workingjubilee

It is now in the queue for this repository.

…jubilee tests: allow trunc/select instructions to be missing On LLVM 20, these instructions already get eliminated, which at least partially satisfies a TODO. I'm not talented enough at using FileCheck to try and constrain this further, but if we really want to we could copy an LLVM 20 specific version of this test that would restore it to being CHECK-NEXT: insertvalue ... `@rustbot` label: +llvm-main r? `@DianQK`

…fee1-dead Rollup of 8 pull requests Successful merges: - rust-lang#128961 (Fix rust-lang#128930: Print documentation of CLI options missing their arg) - rust-lang#129073 (Relate receiver invariantly in method probe for `Mode::Path`) - rust-lang#129674 (Add new_cyclic_in for Rc and Arc) - rust-lang#130201 (Encode `coroutine_by_move_body_def_id` in crate metadata) - rust-lang#130275 (Don't call `extern_crate` when local crate name is the same as a dependency and we have a trait error) - rust-lang#130440 (Don't ICE in `opaque_hidden_inferred_bound` lint for RPITIT in trait with no default method body) - rust-lang#130454 (tests: allow trunc/select instructions to be missing) - rust-lang#130458 (`rustc_codegen_ssa` cleanups) r? `@ghost` `@rustbot` modify labels: rollup

fee1-dead · 2024-09-17T09:45:36Z

@bors r-

#130461 (comment)

fee1-dead · 2024-09-17T09:45:58Z

@bors rollup=iffy

@rustbot

On LLVM 20, these instructions already get eliminated, which at least partially satisfies a TODO. I'm not talented enough at using FileCheck to try and constrain this further, but if we really want to we could copy an LLVM 20 specific version of this test that would restore it to being CHECK-NEXT: insertvalue ... @rustbot label: +llvm-main

durin42 · 2024-09-17T13:05:19Z

Alright, it's just different enough on LLVM 19 that we'll just do what we did before for 19, and the new thing for LLVM 20.

I think this should be fine to rollup, as it's pretty well unchanged for LLVM 19.

workingjubilee · 2024-09-17T23:27:27Z

Alas!

@bors r+

bors · 2024-09-17T23:27:30Z

📌 Commit 9692513 has been approved by workingjubilee

It is now in the queue for this repository.

DianQK · 2024-09-17T23:51:17Z

Thanks!
BTW, this will probably be fixed by #129931.

bors · 2024-09-18T01:27:29Z

⌛ Testing commit 9692513 with merge 60c3673...

bors · 2024-09-18T04:00:44Z

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 60c3673 to master...

rust-timer · 2024-09-18T05:15:00Z

Finished benchmarking commit (60c3673): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.5%, secondary 3.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.5%	[1.5%, 1.5%]	1
Regressions ❌ (secondary)	3.8%	[3.8%, 3.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[1.5%, 1.5%]	1

Cycles

Results (primary 0.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.8%, 0.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.8%	[0.8%, 0.8%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 767.556s -> 768.777s (0.16%)
Artifact size: 341.28 MiB -> 341.28 MiB (0.00%)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) labels Sep 17, 2024

rustbot assigned cuviper Sep 17, 2024

rustbot assigned cjgillot and unassigned cuviper Sep 17, 2024

workingjubilee reviewed Sep 17, 2024

View reviewed changes

tests/codegen/try_question_mark_nop.rs Outdated Show resolved Hide resolved

durin42 force-pushed the llvm-20-notrunc branch 3 times, most recently from 04a9e39 to 978f10d Compare September 17, 2024 01:53

workingjubilee reviewed Sep 17, 2024

View reviewed changes

bors 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 Sep 17, 2024

fee1-dead mentioned this pull request Sep 17, 2024

Rollup of 8 pull requests #130461

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 17, 2024

durin42 force-pushed the llvm-20-notrunc branch from 978f10d to 9692513 Compare September 17, 2024 13:04

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 17, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 17, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2024

bors merged commit 60c3673 into rust-lang:master Sep 18, 2024
7 checks passed

rustbot added this to the 1.83.0 milestone Sep 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: allow trunc/select instructions to be missing #130454

tests: allow trunc/select instructions to be missing #130454

durin42 commented Sep 17, 2024

rustbot commented Sep 17, 2024

durin42 commented Sep 17, 2024

durin42 commented Sep 17, 2024

workingjubilee Sep 17, 2024 •

edited

Loading

durin42 Sep 17, 2024

workingjubilee Sep 17, 2024

workingjubilee commented Sep 17, 2024

bors commented Sep 17, 2024

fee1-dead commented Sep 17, 2024

fee1-dead commented Sep 17, 2024

durin42 commented Sep 17, 2024

workingjubilee commented Sep 17, 2024

bors commented Sep 17, 2024

DianQK commented Sep 17, 2024

bors commented Sep 18, 2024

bors commented Sep 18, 2024

rust-timer commented Sep 18, 2024

tests: allow trunc/select instructions to be missing #130454

tests: allow trunc/select instructions to be missing #130454

Conversation

durin42 commented Sep 17, 2024

rustbot commented Sep 17, 2024

durin42 commented Sep 17, 2024

durin42 commented Sep 17, 2024

workingjubilee Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

durin42 Sep 17, 2024

Choose a reason for hiding this comment

workingjubilee Sep 17, 2024

Choose a reason for hiding this comment

workingjubilee commented Sep 17, 2024

bors commented Sep 17, 2024

fee1-dead commented Sep 17, 2024

fee1-dead commented Sep 17, 2024

durin42 commented Sep 17, 2024

workingjubilee commented Sep 17, 2024

bors commented Sep 17, 2024

DianQK commented Sep 17, 2024

bors commented Sep 18, 2024

bors commented Sep 18, 2024

rust-timer commented Sep 18, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

workingjubilee Sep 17, 2024 •

edited

Loading