diff options
| author | Thomas Lively <tlively@google.com> | 2024-12-17 23:06:03 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-12-18 07:06:03 +0000 |
| commit | d444abdc9fcee98715813f03e28aa7070879c414 (patch) | |
| tree | 79e074285c57490d15cf34d4f1c377cd89da88eb /src/passes/Heap2Local.cpp | |
| parent | 7c8cd2f4a51213964f6f1ec11e54d2b6e721cdaf (diff) | |
| download | binaryen-d444abdc9fcee98715813f03e28aa7070879c414.tar.gz binaryen-d444abdc9fcee98715813f03e28aa7070879c414.tar.bz2 binaryen-d444abdc9fcee98715813f03e28aa7070879c414.zip | |
Handle atomic accesses in Heap2Local (#7158)
Heap2Local replaces gets and sets of non-escaping heap allocations with
gets and sets of locals. Since the accessed data does not escape, it
cannot be used to directly synchronize with other threads, so this
optimization is generally safe even in the presence of shared structs
and atomic struct accesses. The only caveat is that sequentially
consistent accesses additionally participate in the global ordering of
sequentially consistent operations, and that effect on the global
ordering cannot be removed. Insert seqcst fences to maintain this global
synchronization when removing sequentially consistent gets and sets.
Diffstat (limited to 'src/passes/Heap2Local.cpp')
| -rw-r--r-- | src/passes/Heap2Local.cpp | 22 |
1 files changed, 19 insertions, 3 deletions
diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp index c31bc8436..a6223d4fc 100644 --- a/src/passes/Heap2Local.cpp +++ b/src/passes/Heap2Local.cpp @@ -839,9 +839,19 @@ struct Struct2Local : PostWalker<Struct2Local> { // Drop the ref (leaving it to other opts to remove, when possible), and // write the data to the local instead of the heap allocation. - replaceCurrent(builder.makeSequence( + auto* replacement = builder.makeSequence( builder.makeDrop(curr->ref), - builder.makeLocalSet(localIndexes[curr->index], curr->value))); + builder.makeLocalSet(localIndexes[curr->index], curr->value)); + + // This struct.set cannot possibly synchronize with other threads via the + // read value, since the struct never escapes this function. But if the set + // is sequentially consistent, it still participates in the global order of + // sequentially consistent operations. Preserve this effect on the global + // ordering by inserting a fence. + if (curr->order == MemoryOrder::SeqCst) { + replacement = builder.blockify(replacement, builder.makeAtomicFence()); + } + replaceCurrent(replacement); } void visitStructGet(StructGet* curr) { @@ -873,7 +883,13 @@ struct Struct2Local : PostWalker<Struct2Local> { // general. However, signed gets make that more complicated, so leave this // for other opts to handle. value = Bits::makePackedFieldGet(value, field, curr->signed_, wasm); - replaceCurrent(builder.makeSequence(builder.makeDrop(curr->ref), value)); + auto* replacement = builder.blockify(builder.makeDrop(curr->ref)); + // See the note on seqcst struct.set. It is ok to insert the fence before + // the value here since we know the value is just a local.get. + if (curr->order == MemoryOrder::SeqCst) { + replacement = builder.blockify(replacement, builder.makeAtomicFence()); + } + replaceCurrent(builder.blockify(replacement, value)); } }; |