codegen: Never let optimistic effects declassify a julia.safepoint call

[IanButterworth]

I saw the revert #62697 and hadn't seen #62613 until now. This seems to be a fix based on the tests?

---

Note: everything below this line was written with generative AI (Claude).

Since #61394, calls to :consistent+:effect_free functions carry optimistic
memory effects (e.g. memory(argmem: read)) so pre-GC LLVM passes can optimize
around them, relying on LateLowerGCFrame stripping those effects before its
safepoint analysis. That made rooting correctness depend on pass ordering
alone: had LocalScan ever seen the un-stripped attributes, it would have
classified those calls as non-safepoints and dropped the roots of values live
across them — the unsoundness identified in #62613.

Changes

• LocalScan now refuses to treat any call carrying "julia.safepoint" (on
  the call site or the callee) as a non-safepoint, whatever memory effects it
  advertises. Rooting no longer depends on the strip having run first.
• The strip now also matches call-site-only attributes, as emitted when the
  callee is a definition rather than a declaration (single-module AOT), where
  the previous callee-only condition missed it.

Tests

test/llvmpasses/late-lower-gc.ll gains six cases:

Case	Guards against
memory(argmem: read) on the callee declaration	the LocalScan skip quoted in #62613 (JIT path)
attributes on the call site only	the AOT path missed by the old strip condition
memory(read)	the onlyReadsMemory() arm of both checks
indirect call with call-site attributes	the unknown-callee path
readnone on the gcstack argument is stripped	post-GC passes assuming the callee never reads pgcstack (the frame link store must stay visible)
negative control: readonly helper without "julia.safepoint" gets no frame	over-conservatism; keeps the non-safepoint whitelist effective

test/llvmpasses/safepoint-effects-rooting.jl is an end-to-end test through
real codegen with two FileCheck prefixes: the unoptimized run asserts the
optimistic attributes are actually emitted (so the test cannot silently go
stale if effects inference or attribute emission changes), and the optimized
run asserts a tracked value live across such a call keeps its GC-frame root
through the full -O2 pipeline.

Mutation-tested: disabling the LocalScan gate fails every positive case
(the end-to-end test fails on rooting, not on attribute emission); disabling
only the strip fails exactly the readnone-removal case while all rooting
cases still pass on the gate alone.

On the other concerns raised in #62613

• nosync: add_fn_attrs_for_effects never emits nosync, and the JIT
  pipeline has no PostOrderFunctionAttrs (only InferFunctionAttrsPass,
  which is TLI-libfunc-only), so nothing infers it; per LangRef, memory(...)
  does not imply nosync. Atomics and fences still cannot be reordered
  across these calls.
• WeakRef hoisting (per the consensus in the issue thread): loading
  .value establishes a strong root — LateLowerGCFrame roots the loaded
  SSA value like any other tracked value, and since .value is a mutable
  field, the isLoadFromImmut refinement (rooted-by-parent) can never apply
  to it, so the loaded value always gets its own root.
• Finalizers: their ordering is unspecified and the Julia-level optimizer
  already deletes and reorders :effect_free calls; programs that
  synchronize with finalizers do so through locks, and lock operations are
  never :effect_free, so those remain full barriers.
• The optimistic attributes are only emitted for pointer-free signatures
  (the has_user_ptr gate), so everything LLVM may derive from them is a
  subset of what :consistent+:effect_free already permit at the Julia
  level — except GC rooting, which LateLowerGCFrame now guards
  unconditionally.

A follow-up worth doing regardless (relevant for a future concurrent GC):
emit WeakRef.value loads as atomic monotonic.

Alternative to #62697.
Fixes #62613

[gbaraldi]

This is not how this should be fixed. The issues are not specific to lower GC

[IanButterworth]

Is it worth merging the tests as broken? or are we committed to the revert?

[vtjnash]

We are committed to revert because there's an issue identified, though we may re-land as soon as those are resolved.