Skip to content

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Aug 6, 2025

Notably this has the IRCE fix

Notably this has the IRCE fix
@gbaraldi gbaraldi requested a review from giordano as a code owner August 6, 2025 22:35
@giordano
Copy link
Member

giordano commented Aug 6, 2025

Notably this has the IRCE fix

I presume this refers to #48918? Is that issue fully fixed or something like #59129 is also required? Any regression tests we can add here?

@giordano
Copy link
Member

giordano commented Aug 6, 2025

Analyzegc is unhappy https://buildkite.com/julialang/julia-master/builds/49705#01988187-7f07-4890-b9af-a7b09b133eac/1171-1767

    ANALYZE src/clang-sagc-gc-stock
/cache/build/tester-amdci5-12/julialang/julia-master/src/gc-stock.c:3080:9: error: Calling potential safepoint as SimpleFunctionCall from function annotated JL_NOTSAFEPOINT [julia.GCChecker]
 3080 |         gc_mark_loop(ptls);
      |         ^~~~~~~~~~~~~~~~~~

@Keno
Copy link
Member

Keno commented Aug 6, 2025

Is that issue fully fixed or something like #59129 is also required?

Would also fix it, but this fixes the observed crash, but in the case where the crash happens, the original semantics will cause the value to be rooted longer than intended.

@gbaraldi gbaraldi requested a review from fingolfin as a code owner August 7, 2025 14:27
@Keno Keno merged commit f701768 into master Aug 7, 2025
7 checks passed
@Keno Keno deleted the gb/llvm2018 branch August 7, 2025 20:05
@KristofferC
Copy link
Member

Would it be good to have an in repo test of the issue that this bump fixed? So we notice if it regresses.

@gbaraldi
Copy link
Member Author

gbaraldi commented Aug 7, 2025

We never got a really small reproducer sadly. It needs at least a big chunk of staticarrays

@Keno
Copy link
Member

Keno commented Aug 7, 2025

Yeah, it's somewhat hard to come up with a small reproducer. It needs at least control flow outside a @GC.preserve region (or something that LLVM will turn into that) and an IRCE-eligible loop. It should be possible to write such a reproducer now that we understand the issue, but I don't have one.

@gbaraldi
Copy link
Member Author

gbaraldi commented Aug 7, 2025

We can write an LLVM test and run it locally but keno's patch tests this upstream

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.

4 participants