Reland: Improve performance of global code by emitting fewer atomic barriers.

[maleadt]

Re-land #47578 which got reverted because of a GC issue on win32. I haven't debugged this, but want to have an open PR so that I don't lose track of this.

Fixes #47561

[vchuravy]

The good news is that it also seems to happen on Linux i686 so we might have a hope to debug it :)

[maleadt]

So this reproduces pretty consistently on 32-bit Linux by just doing runtests.jl LinearAlgebra/special, and I even have an rr trace, but it's not very helpful. I'll probably need a debug build.

[maleadt]

Finally caught this in a trace with debug info: https://julialang-dumps.s3.amazonaws.com/reports/2022-12-21T12-04-45-maleadt.tar.zst. Note that this is a 32-bit process, so you either need to replay this with on a 32-bit system, or use a build of rr that supports replaying 32-bit processes (i.e. not the one from rr_jll).

[maleadt]

Some digging in rr. This crashes when accessing an invalid field of an object:

(rr) p *pnew_obj
$3 = (jl_value_t *) 0x13

(rr) p (jl_value_t*)parent
$14 = (jl_value_t *) 0xb1a97b5c

(rr) call jl_((jl_value_t*)parent)
LinearAlgebra.QRCompactWY{Float64, Array{Float64, 2}, Array{Float64, 2}}(factors=#<intrinsic #29 sle_int>, T=#<19>)

Note the invalid factors=#<intrinsic #29 sle_int> field, which should be a matrix instead. Setting a watchpoint on that field brings us to the place it gets overwritten:

Old value = 19
New value = -1359114816
0xe7da775e in julia__useref_setindex!_14279 () at compiler/ssair/ir.jl:466
466     @noinline function _useref_setindex!(@nospecialize(stmt), op::Int, @nospecialize(v))

(rr) call jl_((jl_value_t*)0xb1a97b5c)
Core.GotoIfNot(cond=SSAValue(51), dest=-1359114816)

Not only does the field get overwritten here, the object is entirely different (i.e. the object's header is different). I'm not sure whether that means that the QRCompactWY wasn't properly rooted, or how we end up with a mix of two objects. Breaking on where the object's header gets set to the QRCompactWY type doesn't give me an interesting backtrace:

Old value = 2980674488
New value = 2926770304
0xb5a88c1c in ?? ()

# manual expansion of jl_typeof (is what I set a watchpoint on)
(rr) call jl_((jl_astaggedvalue((jl_value_t*)0xb1a97b5c))->header  & ~(uintptr_t)15)
LinearAlgebra.QRCompactWY{Float64, Array{Float64, 2}, Array{Float64, 2}}

(rr) bt
#0  0xb5a88c1c in ?? ()
#1  0x00000000 in ?? ()

[maleadt]

So this consistently crashes during LinearAlgebra/special with the above GC corruption when removing the atomic barrier between every toplevel instruction. To be sure, the latest commits puts it back, showing that the issue isn't caused by either switching to monotonic ordering, or by late-inserting barriers before calls.

@vtjnash Do you have any thoughts how this could be related? Could you explain the full purpose of these barriers? I thought it was only required to ensure a correct world age, so I'm not sure how removing those loads could result in GC corruption.

[giordano]

Bump. It'd be a bit unfortunate to have another minor version with this bug 🙂

[maleadt]

Rebased. Looks like the error doesn't happen anymore? I couldn't try locally, because this time even a non-debug build failed because of address space exhaustion.

EDIT: using the i686 Buildkite artifact, I'm running the LinearAlgebra/special testsuite a bunch of times right now, and it hasn't failed yet. Looking good?

[vtjnash]

Why are we relaxing this from acquire to monotonic?

[maleadt]

I think you suggested that?

[vtjnash]

I don't remember

[maleadt]

Do you want me to remove that part?

[maleadt]

Backporting these changes on top of 1.9 still results in segfaults in i686 while running LinearAlgebra/special...

@vtjnash if you want, I can try generating an rr trace for you to take a look (as I'm slightly concerned that the underlying issue may still be present here; and it would also be useful to have this on 1.9).

[vtjnash]

Yes, that would be helpful