remove fence that GC change made unnecessary

[oscardssmith]

@gbaraldi and I think that this fence can probably be removed now that #55223 is merged. This should slightly expand the set of cases where the Memory is removable.

[gbaraldi]

I'm still spooked about DSE being able to delete stores into our objects, but LGTM

[vtjnash]

I thought we already have this fence in our safepoint lowering?

[gbaraldi]

This fence is not the safepoint one. It's specifically for the length field of a memory, which if it is unused can be DSEd out. We used to use it during sweeping to decrement the GC length, but not anymore.

[oscardssmith]

@nanosoldier runtests()

[vtjnash]

We use it during marking, to decide what memory to access

[gbaraldi]

So that's where we have some questions. From what we've seen this was only an issue if the memory was allocated and was instantly dead (i.e never marked). Which might be wrong, though this is what we were discussing yesterday in relation to initialization.

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Report summary

❗ Packages that crashed

2 packages crashed only on the current version.

• A segmentation fault happened: 2 packages

13 packages crashed on the previous version too.

✖ Packages that failed

20 packages failed only on the current version.

• Package has test failures: 5 packages
• Tests became inactive: 2 packages
• Test duration exceeded the time limit: 13 packages

1311 packages failed on the previous version too.

✔ Packages that passed tests

19 packages passed tests only on the current version.

• Other: 19 packages

5212 packages passed tests on the previous version too.

~ Packages that at least loaded

11 packages successfully loaded only on the current version.

• Other: 11 packages

2713 packages successfully loaded on the previous version too.

➖ Packages that were skipped altogether

913 packages were skipped on the previous version too.

[oscardssmith]

The pkgeval failures look fairly unrelated, but I think @vtjnash and @gbaraldi are most likely to be able to figure out whether this is valid or not.

[oscardssmith]

From looking at this with Jeff, we realized that GC-safepoints have a CST fence, which should make this safe (since that requires LLVM not to delay the store of a living object beyond the fence).

[gbaraldi]

Right, but does LLVM treat other things that are safepoints, i.e julia function calls and friends as seq-cst fences. (I believe it does) so I think merging is fine.

[oscardssmith]

I think the situation here is somewhat complicated. Currently, I believe this optimization is safe since LLVM believes that all Julia function calls may allias all memory. Interestingly, since calling a julia function may run GC this is almost true, in that any function call may read any live memory (in the mark phase), but will not read any dead memory. In some ways, this would be a lot cleaner if the safepoint on functions calls was added by the caller rather than the callee.

[KristofferC]

@nanosoldier runtests(["SimpleLooper", "WeaklyHard", "EarlyStopping", "MbedTLS", "TuePlots", "FaultDetectionTools", "Andes", "AstroRepresentations", "RandomLinearAlgebraSolvers", "Optim", "PlotRNA", "MomentMatching", "DynamicMovementPrimitives", "DynamicalSystems", "Trebuchet", "ModelOrderReductionToolkit", "ViscousFlow", "Yunir", "StirredReactor", "Survey", "SoilWater_ToolBox", "Chamber"])

[nanosoldier]

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Report summary

✖ Packages that failed

2 packages failed only on the current version.

• Tests became inactive: 2 packages

2 packages failed on the previous version too.

✔ Packages that passed tests

12 packages passed tests on the previous version too.

~ Packages that at least loaded

6 packages successfully loaded on the previous version too.