Allow atomic operations on non-shared memory in SafeHeap#8890
Conversation
SafeHeap needs to generate atomic helper functions even if the memory is not shared, because atomic instructions can still be used. TAG=agy CONV=75c799ef-6dca-4712-8ffe-d8d1dfaffd0b
| bool | ||
| isPossibleAtomicOperation(Index align, Index bytes, bool shared, Type type) { | ||
| return align == bytes && shared && type.isInteger(); | ||
| isPossibleAtomicOperation(Index align, Index bytes, bool /*shared*/, Type type) { |
There was a problem hiding this comment.
If any human or bot gets a chance to look at this, please remove the newly unused parameter.
|
Yeah I was discussing this with @brendandahl and we think the first fix to the emscripten breakage that prompted this (see triage_20260708_084511.html on the internal x20 link) would be to ensure that we link with |
|
When libraries using atomic are compiled with atomics enabled, those atomics are not lowered away. Then we allow those libraries to be linked into single-threaded applications, which will not have shared memories. Presumably SafeHeap will want to instrument the resulting atomic memory accesses to unshared memories the same as it would any other memory access, so I think this PR is a correct fix. |
|
True; but IIRC there were some cases where we do lower atomics away after linking (for browsers that don't support atomic at all). I wasn't sure whether this case (where were "incorrectly" failling to use -pthread together with openMP) was supposed to have that happen. Or maybe it would have happened after this particular pass. |
|
Going back to the change itself, this just means that we'll generate atomic helper functions in many cases where we don't today, but that seems fine. I can go ahead and fix this up, write a test, etc. |
Oh interesting, I wasn't aware of that. I agree this change is correct regardless of that, though. |
SafeHeap needs to generate atomic helper functions even if the memory
is not shared, because atomic instructions can still be used.
TAG=agy
CONV=75c799ef-6dca-4712-8ffe-d8d1dfaffd0b