Skip to content

Conversation

@Enselic
Copy link
Member

@Enselic Enselic commented Dec 4, 2025

Closes #60044 which has one 👍 and one ❤️ vote and just E-needs-test.

@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 Dec 4, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2025

r? @Mark-Simulacrum

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

@clubby789
Copy link
Contributor

I think this should be a codegen-llvm test as it's not backend-specific

@Enselic
Copy link
Member Author

Enselic commented Dec 9, 2025

Good call. That deserves consideration.

Here is what the correct LLVM IR currently looks like:

; Function Attrs: mustprogress nofree norecurse nounwind nonlazybind willreturn memory(readwrite, argmem: none, inaccessiblemem: write)
define noundef range(i64 1, 0) i64 @some_non_zero_from_atomic_get() unnamed_addr #0 {
start:
  %0 = load atomic i64, ptr @_ZN38some_non_zero_from_atomic_optimization1X17h41fcdb7c72ef9763E monotonic, align 8
  %1 = icmp ne i64 %0, 0
  tail call void @llvm.assume(i1 %1)
  ret i64 %0
}
Full LLVM IR
; ModuleID = 'some_non_zero_from_atomic_optimization.ca4055969c8517ab-cgu.0'
source_filename = "some_non_zero_from_atomic_optimization.ca4055969c8517ab-cgu.0"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

@_ZN38some_non_zero_from_atomic_optimization1X17h41fcdb7c72ef9763E = local_unnamed_addr global [8 x i8] c"\07\00\00\00\00\00\00\00", align 8

@some_non_zero_from_atomic_get2 = unnamed_addr alias i64 (), ptr @some_non_zero_from_atomic_get

; Function Attrs: mustprogress nofree norecurse nounwind nonlazybind willreturn memory(readwrite, argmem: none, inaccessiblemem: write)
define noundef range(i64 1, 0) i64 @some_non_zero_from_atomic_get() unnamed_addr #0 {
start:
  %0 = load atomic i64, ptr @_ZN38some_non_zero_from_atomic_optimization1X17h41fcdb7c72ef9763E monotonic, align 8
  %1 = icmp ne i64 %0, 0
  tail call void @llvm.assume(i1 %1)
  ret i64 %0
}

; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write)
declare void @llvm.assume(i1 noundef) #1

attributes #0 = { mustprogress nofree norecurse nounwind nonlazybind willreturn memory(readwrite, argmem: none, inaccessiblemem: write) "probe-stack"="inline-asm" "target-cpu"="x86-64" }
attributes #1 = { mustprogress nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) }

!llvm.module.flags = !{!0, !1}
!llvm.ident = !{!2}

!0 = !{i32 8, !"PIC Level", i32 2}
!1 = !{i32 2, !"RtLibUseGOT", i32 1}
!2 = !{!"rustc version 1.91.1 (ed61e7d7e 2025-11-07)"}

As you can see, the LLVM IR is considerably more elaborate and is likely to change over time as we adjust codegen and adapt to new LLVM versions. So an LLVM IR version of this test risks becoming a maintenance burden that will steal time from doing other improvements.

Besides, it does not seem impossible that this optimization could regress on the LLVM side, which an LLVM IR based test would not catch.

So my recommendation is to go ahead with this test, because it tests what actually matters (the end result) and that is unlikely to ever change.

That said, I can certainly turn this into an LLVM test if that is considered mandatory for merge. We can wait and see see what others think perhaps.

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

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missed NonZero optimization opportunity

4 participants