Skip to content

Conversation

oscardssmith
Copy link
Member

This is a revival of #43738 with a lot of changes. The biggest by far is that with this version of the PR, I'm trying to get the BoundsError to semantically not escape the input argument so that LLVM escape analysis doesn't see bounds error construction as a potential escape (motivated by #55913). That said, this will likely significantly increase IR size, so I'm not sure if it is worth it or if this is a good tradeoff...

@oscardssmith oscardssmith requested a review from aviatesk October 15, 2024 03:58
@nsajko nsajko added the arrays [a, r, r, a, y, s] label Oct 15, 2024
@giordano giordano marked this pull request as draft October 15, 2024 11:22
@giordano giordano changed the title draft: Make BoundsError lazy and move Memory boundscheck to Julia Make BoundsError lazy and move Memory boundscheck to Julia Oct 15, 2024
@giordano
Copy link
Member

I tried to revive this PR (I can push my local changes if helpful, at least to resolve the merge conflict), but the problem is that getfield doesn't throw a BoundsError with the summary type introduced here:

Test Failed at /cache/build/tester-demeter6-11/julialang/julia-master/julia-bf20618f35/share/julia/test/core.jl:1316
  Expression: getfield(z, -1)
    Expected: BoundsError(Core.Summarized("Complex{Int64}", nothing, Complex{Int64}), -1)
      Thrown: BoundsError(3 + 4im, -1)
      BoundsError: attempt to access Complex{Int64} at index [-1]
      Stacktrace:
       [1] macro expansion
         @ /cache/build/tester-demeter6-11/julialang/julia-master/julia-bf20618f35/share/julia/test/core.jl:1316 [inlined]
       [2] macro expansion
         @ /cache/build/tester-demeter6-11/julialang/julia-master/julia-bf20618f35/share/julia/stdlib/v1.12/Test/src/Test.jl:773 [inlined]
       [3] top-level scope
         @ /cache/build/tester-demeter6-11/julialang/julia-master/julia-bf20618f35/share/julia/test/core.jl:1316

I don't quite know how to fix that, I don't know where getfield is defined and throws the error (I presume it's somewhere in the C++ code?)

@oscardssmith
Copy link
Member Author

see emit_getfield_unknownidx in cgutils.cpp

@giordano giordano force-pushed the os/lazy-bounds-error branch from bf20618 to 195d8a1 Compare June 9, 2025 00:12
@giordano
Copy link
Member

giordano commented Jun 9, 2025

In the meantime I pushed the commits to resolve the merge conflicts, but I didn't have a chance to look into the getfield issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants