Closed
Bug 1154714
Opened 9 years ago
Closed 9 years ago
Inlining of Atomics.store should not consider the inline return context
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
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•9 years ago
|
||
Er, that would be bug 1077305.
Assignee | ||
Comment 2•9 years ago
|
||
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 3•9 years ago
|
||
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+
Assignee | ||
Comment 5•9 years ago
|
||
The review comment has been addressed in that patch :)
https://hg.mozilla.org/mozilla-central/rev/3ff53c56babc
Status: ASSIGNED → RESOLVED
Closed: 9 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.
Description
•