Skip to content

Conversation

@KristofferC
Copy link
Member

@KristofferC KristofferC commented Jan 14, 2026

During incremental compaction a forwarded NewSSAValue with a negative id was treated like an SSAValue, producing SSAValue(-n) and tripping renumber_ssa2 bounds checks. Only convert NewSSAValue to SSAValue when the id is positive, so new_new_nodes references remain valid.

🤖 AI fix for #57827. Putting up for consideration.

@KristofferC KristofferC force-pushed the kc/negative_ssa_keep branch 2 times, most recently from 4fe71de to 4a5e138 Compare January 14, 2026 21:32
Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

I think the fix is probably correct.

@KristofferC KristofferC added backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 labels Jan 15, 2026
@DilumAluthge DilumAluthge mentioned this pull request Jan 15, 2026
40 tasks
@Keno
Copy link
Member

Keno commented Jan 22, 2026

Could also use an IR-level test.

@KristofferC KristofferC mentioned this pull request Jan 26, 2026
43 tasks
@KristofferC
Copy link
Member Author

julia> g = (acc, x) -> ifelse(x > acc[1], (x,), (acc[1],))
#2 (generic function with 1 method)

julia> rf = Base.BottomRF(g)
Base.BottomRF{var"#2#3"}(var"#2#3"())

julia> optimize_until = "CC: COMPACT_2";

# ok
julia> Base.code_ircode(Base._foldl_impl, (typeof(rf), Tuple{Float64}, Vector{Int64}); optimize_until)
1-element Vector{Any}:
54 1 ── %1   = intrinsic Base.sub_int(1, 1)::Int64  iterate
   │    %2   = intrinsic Base.bitcast(Base.UInt, %1)::UInt64

...

julia> optimize_until = "CC: SROA";

# bad
julia> Base.code_ircode(Base._foldl_impl, (typeof(rf), Tuple{Float64}, Vector{Int64}); optimize_until)
ERROR: BoundsError: attempt to access 183-element Vector{Int64} at index [-13]
Stacktrace:
  [1] throw_boundserror(A::Vector{Int64}, I::Tuple{Int64})
    @ Base ./essentials.jl:13
  [2] checkbounds
    @ ./essentials.jl:385 [inlined]
  [3] getindex
    @ ./essentials.jl:964 [inlined]
  [4] renumber_ssa2(val::Core.SSAValue, ssanums::Vector{…}, used_ssas::Vector{…}, new_new_used_ssas::Vector{…}, do_rename_ssa::Bool, mark_refined!::Compiler.Refiner)

@KristofferC
Copy link
Member Author

I didn't manage to produce a pure IR test, but I managed to isolate it into locally defined functions without Base dependencie,s which might be ok for now.

During incremental compaction a forwarded NewSSAValue with a negative id was treated like an SSAValue, producing SSAValue(-n) and tripping renumber_ssa2 bounds checks. Only convert NewSSAValue to SSAValue when the id is positive so new_new_nodes references remain valid.
@aviatesk aviatesk merged commit 7eaf4e8 into master Jan 29, 2026
8 checks passed
@aviatesk aviatesk deleted the kc/negative_ssa_keep branch January 29, 2026 11:14
aviatesk pushed a commit that referenced this pull request Jan 29, 2026
During incremental compaction a forwarded NewSSAValue with a negative id
was treated like an SSAValue, producing SSAValue(-n) and tripping
`renumber_ssa2` bounds checks. Only convert NewSSAValue to SSAValue when
the id is positive, so new_new_nodes references remain valid.

Fixes #57827.
aviatesk pushed a commit that referenced this pull request Jan 29, 2026
During incremental compaction a forwarded NewSSAValue with a negative id
was treated like an SSAValue, producing SSAValue(-n) and tripping
`renumber_ssa2` bounds checks. Only convert NewSSAValue to SSAValue when
the id is positive, so new_new_nodes references remain valid.

Fixes #57827.
@aviatesk aviatesk removed backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 labels Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants