Closed Bug 1225026 Opened 4 years ago Closed 4 years ago

Remove support for atomic operations on Uint8ClampedArray

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: lth, Assigned: lth)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

SpiderMonkey supported atomics on Uint8ClampedArray since day 1, because it was easy to implement.  (Well, on SharedUint8ClampedArray, but that is going away in bug 1176214 in favor of just Uint8ClampedArray on SharedArrayBuffer.)  However, it's not been favored by the SAB+Atomics working group, and it's not in the spec, and I don't think it's going to come back.

So we should remove this from the engine (after 1176214 has landed), in order to be spec compliant.  Mostly I think this is work in builtin/AtomicsObject.cpp, because the JIT will not optimize atomic operations on Uint8ClampedArray, but it will be good to check the code paths deeper in the JIT to make sure no code is sitting around by mistake.
This should not further complicate the tag checking in jitted code, BTW: atomic operations are only open-coded if we know that the type of the array is something we care about.  It's checking the sharedness of the array that we allow to be deferred to run-time if it's unknown.
(In reply to Lars T Hansen [:lth] from comment #0)
> 
> Mostly I think this is work in
> builtin/AtomicsObject.cpp, because the JIT will not optimize atomic
> operations on Uint8ClampedArray, but it will be good to check the code paths
> deeper in the JIT to make sure no code is sitting around by mistake.

In fact, this turned out to be mostly about removing code from the JIT.
Attachment #8694841 - Flags: review?(nicolas.b.pierron)
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Comment on attachment 8694841 [details] [diff] [review]
remove support for atomics on Uint8ClampedArray

Review of attachment 8694841 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for adding test cases. :)
Attachment #8694841 - Flags: review?(nicolas.b.pierron) → review+
https://hg.mozilla.org/mozilla-central/rev/b6eea9eddc4a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.