Inlining of Atomics.store should not consider the inline return context

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: lth, Assigned: lth)

Tracking

(Blocks: 1 bug)

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
The Atomics methods are inlined if they meet certain preconditions, and those preconditions currently always take into account the return type context.  Notably for Uint32 the return type context must be double (see also bug 1077035).  In any case, for Atomics.store the return type context is irrelevant, because the value that's returned from Atomics.store is the value that's the third argument, not some value that was read from the array.  So we should not check the return type context.
(Assignee)

Comment 1

3 years ago
Er, that would be bug 1077305.
(Assignee)

Comment 2

3 years ago
Created attachment 8592812 [details] [diff] [review]
Disable checking the inline return type when not relevant

The existing test here, that the inline return type for the Int8..Int32 array types is MIRType_Int32, is almost certainly redundant but I left it in for now, as it's not the focus of this bug.
Attachment #8592812 - Flags: review?(benj)
Comment on attachment 8592812 [details] [diff] [review]
Disable checking the inline return type when not relevant

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

Looks good, sorry for the long review time.

::: js/src/jit/IonBuilder.h
@@ +896,5 @@
> +        DoCheckAtomicResult
> +    };
> +
> +    bool atomicsMeetsPreconditions(CallInfo& callInfo, Scalar::Type* arrayElementType,
> +                                   AtomicCheckResult=DoCheckAtomicResult);

Can you put the in/out argument at the last position, please?
Attachment #8592812 - Flags: review?(benj) → review+

Comment 4

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ff53c56babc
(Assignee)

Comment 5

3 years ago
The review comment has been addressed in that patch :)
https://hg.mozilla.org/mozilla-central/rev/3ff53c56babc
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.