Skip to content

LLVM 23: Adapt codegen test to moved assume#158067

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
TimNN:moved-assume
Jun 26, 2026
Merged

LLVM 23: Adapt codegen test to moved assume#158067
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
TimNN:moved-assume

Conversation

@TimNN

@TimNN TimNN commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

I wasn't completely sure what the important part of this test is - based on the "Make sure we don't create br or select instructions" comment I assume it's that the load is followed ~immediately by the ret, and that the presence or absence of the assume doesn't really matter (it's been moved up a few lines to before the load):

; Function Attrs
define noundef i32 @foo(ptr noalias nofree noundef align 8 captures(none) dereferenceable(16) %x) unnamed_addr #0 { 
start
 %_6 = load ptr, ptr %x, align 8, !nonnull !4, !noundef !4 
 %0 = getelementptr inbounds nuw i8, ptr %x, i64 8 
 %_7 = load ptr, ptr %0, align 8, !nonnull !4, !noundef !4 
 %_8 = icmp ne ptr %_6, %_7 
 tail call void @llvm.assume(i1 %_8) 
 %_14 = getelementptr inbounds nuw i8, ptr %_6, i64 4 
 store ptr %_14, ptr %x, align 8 
 %_4 = load i32, ptr %_6, align 4, !noundef !4 
 ret i32 %_4 
} 

@rustbot label llvm-main

original CI failure

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

rustbot commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

r? @mu001999

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

@rustbot rustbot added the llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) label Jun 18, 2026
@mu001999

mu001999 commented Jun 18, 2026

Copy link
Copy Markdown
Member

r? dianqk (creator of this test)

@rustbot rustbot assigned dianqk and unassigned mu001999 Jun 18, 2026
@rustbot

rustbot commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

dianqk is currently at their maximum review capacity.
They may take a while to respond.

//@ [new] min-llvm-version: 23

// Test for #107681.
// Make sure we don't create `br` or `select` instructions.

This comment was marked as duplicate.

Comment thread tests/codegen-llvm/issues/issue-107681-unwrap_unchecked.rs Outdated
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@dianqk

dianqk commented Jun 25, 2026

Copy link
Copy Markdown
Member

It looks like on LLVM 21 the IR structure is a bit different and the proposed checks with INNER don't work.
Since this test is only testing an optimization, would it make sense to disable it on LLVM 21?

IIRC, they have different layouts, through I don't know why. Anyway, you can just ignore LLVM 21.

@dianqk

dianqk commented Jun 25, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

@rust-bors

rust-bors Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 4691890 has been approved by dianqk

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

@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 25, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 25, 2026
LLVM 23: Adapt codegen test to moved assume

I wasn't completely sure what the important part of this test is - based on the "Make sure we don't create `br` or `select` instructions" comment I assume it's that the `load` is followed ~immediately by the `ret`, and that the presence or absence of the `assume` doesn't really matter (it's been moved up a few lines to before the `load`):

```ll
; Function Attrs
define noundef i32 @foo(ptr noalias nofree noundef align 8 captures(none) dereferenceable(16) %x) unnamed_addr #0 {
start
 %_6 = load ptr, ptr %x, align 8, !nonnull !4, !noundef !4
 %0 = getelementptr inbounds nuw i8, ptr %x, i64 8
 %_7 = load ptr, ptr %0, align 8, !nonnull !4, !noundef !4
 %_8 = icmp ne ptr %_6, %_7
 tail call void @llvm.assume(i1 %_8)
 %_14 = getelementptr inbounds nuw i8, ptr %_6, i64 4
 store ptr %_14, ptr %x, align 8
 %_4 = load i32, ptr %_6, align 4, !noundef !4
 ret i32 %_4
}
 ```

@rustbot label llvm-main

[original CI failure](https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/46608/list?sid=019ed950-c81f-466d-a3bc-2064e6a26719&tab=output)
rust-bors Bot pushed a commit that referenced this pull request Jun 25, 2026
Rollup of 10 pull requests

Successful merges:

 - #158410 (Update LLVM for Mach-O __LINKEDIT alignment fix.)
 - #157397 (cmse: clear padding when crossing the secure boundary)
 - #158036 (Add -Zinstrument-mcount=fentry to -Zinstrument-mcount)
 - #158330 (llvm: use intrinsics for f16, f32 minimum/maximum)
 - #158359 (fix(tests): allow either branch direction in ilog_known_base)
 - #158067 (LLVM 23: Adapt codegen test to moved assume)
 - #158261 (Move part of the target checking for `#[may_dangle]` to the parser)
 - #158358 (Fix invalid E0609 raw pointer deref suggestion inside macros)
 - #158392 (delegation: add tests for defaults and infers in generics)
 - #158394 (Generate synthetic generic args only for delegation's child segment)
@rust-bors rust-bors Bot merged commit 3e2c952 into rust-lang:main Jun 26, 2026
13 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 26, 2026
rust-timer added a commit that referenced this pull request Jun 26, 2026
Rollup merge of #158067 - TimNN:moved-assume, r=dianqk

LLVM 23: Adapt codegen test to moved assume

I wasn't completely sure what the important part of this test is - based on the "Make sure we don't create `br` or `select` instructions" comment I assume it's that the `load` is followed ~immediately by the `ret`, and that the presence or absence of the `assume` doesn't really matter (it's been moved up a few lines to before the `load`):

```ll
; Function Attrs
define noundef i32 @foo(ptr noalias nofree noundef align 8 captures(none) dereferenceable(16) %x) unnamed_addr #0 {
start
 %_6 = load ptr, ptr %x, align 8, !nonnull !4, !noundef !4
 %0 = getelementptr inbounds nuw i8, ptr %x, i64 8
 %_7 = load ptr, ptr %0, align 8, !nonnull !4, !noundef !4
 %_8 = icmp ne ptr %_6, %_7
 tail call void @llvm.assume(i1 %_8)
 %_14 = getelementptr inbounds nuw i8, ptr %_6, i64 4
 store ptr %_14, ptr %x, align 8
 %_4 = load i32, ptr %_6, align 4, !noundef !4
 ret i32 %_4
}
 ```

@rustbot label llvm-main

[original CI failure](https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/46608/list?sid=019ed950-c81f-466d-a3bc-2064e6a26719&tab=output)
rust-bors Bot pushed a commit that referenced this pull request Jun 27, 2026
…athanBrouwer

Revert "LLVM 23: Adapt codegen test to moved assume"

This reverts #158067.
Fixes #158500

The fix that was applied here is not robust against layout randomization, specifically the IR to load through the slice head pointer needs to come after a gepi, and not just a `load ptr, ptr %x`.

Based on the PR discussion, I think it's possible that the `min-llvm-version: 22` was mistakenly added when CI happened to choose the bad layout during the initial merge.

I'm not yet sure what the right fix is, but this is causing about half of all merges to fail right now so a revert seems prudent.

I don't think we have a way to skip tests when randomize-layout is set. The problem here is that we run codegen tests against the globally-configured sysroot, so setting `-Zrandomize-layout` in the test doesn't do anything.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 27, 2026
…-fix, r=JonathanBrouwer

Revert "LLVM 23: Adapt codegen test to moved assume"

This reverts rust-lang#158067.
Fixes rust-lang#158500

The fix that was applied here is not robust against layout randomization, specifically the IR to load through the slice head pointer needs to come after a gepi, and not just a `load ptr, ptr %x`.

Based on the PR discussion, I think it's possible that the `min-llvm-version: 22` was mistakenly added when CI happened to choose the bad layout during the initial merge.

I'm not yet sure what the right fix is, but this is causing about half of all merges to fail right now so a revert seems prudent.

I don't think we have a way to skip tests when randomize-layout is set. The problem here is that we run codegen tests against the globally-configured sysroot, so setting `-Zrandomize-layout` in the test doesn't do anything.
rust-bors Bot pushed a commit that referenced this pull request Jun 27, 2026
…athanBrouwer

Revert "LLVM 23: Adapt codegen test to moved assume"

This reverts #158067.
Fixes #158500

The fix that was applied here is not robust against layout randomization, specifically the IR to load through the slice head pointer needs to come after a gepi, and not just a `load ptr, ptr %x`.

Based on the PR discussion, I think it's possible that the `min-llvm-version: 22` was mistakenly added when CI happened to choose the bad layout during the initial merge.

I'm not yet sure what the right fix is, but this is causing about half of all merges to fail right now so a revert seems prudent.

I don't think we have a way to skip tests when randomize-layout is set. The problem here is that we run codegen tests against the globally-configured sysroot, so setting `-Zrandomize-layout` in the test doesn't do anything.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants