Closed
Bug 1208876
Opened 9 years ago
Closed 9 years ago
add 64 bit operations to MacroAssembler-none
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla44
People
(Reporter: stevensn, Assigned: stevensn)
References
Details
Attachments
(1 file, 1 obsolete file)
8.42 KB,
patch
|
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Patches in bug 774364 moved/added some 64 bit operation functions to the architecture MacroAssembler-XXX classes but missed MacroAssembler-none.
Assignee | ||
Comment 1•9 years ago
|
||
Add missing functions to avoid the compiler errors with no jit.
Attachment #8666494 -
Flags: review?(sstangl)
Comment 2•9 years ago
|
||
Comment on attachment 8666494 [details] [diff] [review]
Add functions to MacroAssembler-none
Review of attachment 8666494 [details] [diff] [review]:
-----------------------------------------------------------------
Stealing review, as it's trivial. Thanks!
Attachment #8666494 -
Flags: review?(sstangl) → review+
Assignee | ||
Comment 3•9 years ago
|
||
Assignee: nobody → steve
Keywords: checkin-needed
Comment 4•9 years ago
|
||
Passing by: wrong bug reference in the commit message.
Comment 5•9 years ago
|
||
Hi, this does not apply cleanly:
patching file js/src/jit/none/MacroAssembler-none.h
Hunk #1 FAILED at 232
Hunk #2 FAILED at 253
Hunk #3 FAILED at 279
Hunk #4 FAILED at 392
4 out of 4 hunks FAILED -- saving rejects to file js/src/jit/none/MacroAssembler-none.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh bug1208876.patch
Flags: needinfo?(steve)
Keywords: checkin-needed
Assignee | ||
Comment 6•9 years ago
|
||
Sorry, I mistyped the bug number in the commit comment. This has been applied
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa7e651a038a
https://hg.mozilla.org/mozilla-central/rev/aa7e651a038a
Flags: needinfo?(steve)
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 7•9 years ago
|
||
Updating the comment in the patch to include the proper bug #
Attachment #8666494 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
status-firefox43:
--- → affected
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none
Approval Request Comment
[Feature/regressing bug #]:774364
[User impact if declined]: non-ion builds (tier 3) will fail
[Describe test coverage new/current, TreeHerder]: Tested on m-c
[Risks and why]: low risk not part of the build for tier-1 platforms
[String/UUID change made/needed]:none
Attachment #8669357 -
Flags: approval-mozilla-aurora?
Comment 9•9 years ago
|
||
Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none
Build issue, OK to uplift to aurora.
Attachment #8669357 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•9 years ago
|
||
Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none
Can this be added to firefox-43 prior to its release, too? There are still some platforms that build firefox, that need to have jit disabled.
Attachment #8669357 -
Flags: approval-mozilla-beta?
Comment 11•9 years ago
|
||
Hmm, this was approved to land in 43 while it was still in the aurora channel. I'm not sure if it landed. It may have.
Kwierso or tomcat, did this make it into 43 already?
Flags: needinfo?(wkocher)
Flags: needinfo?(cbook)
I don't believe it has been uplifted to 43, likely because the target milestone was never set.
Flags: needinfo?(wkocher)
Target Milestone: --- → mozilla44
Comment 13•9 years ago
|
||
Benjamin, or jandem, what do you think about the risk of uplifting this to beta? We are heading into beta 8 at this point so it's a bit late in the cycle.
Flags: needinfo?(jdemooij)
Flags: needinfo?(benj)
Comment 14•9 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #13)
> Benjamin, or jandem, what do you think about the risk of uplifting this to
> beta?
The patch is NPOTB (none of our official builds use this configuration) so this is no-risk. Technically it doesn't even need approval and can land with a=npotb I think.
Flags: needinfo?(jdemooij)
Flags: needinfo?(benj)
Comment 15•9 years ago
|
||
Oh of course. Thanks!
Comment 16•9 years ago
|
||
Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none
Not part of the build, ok to uplift to beta
Attachment #8669357 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 17•9 years ago
|
||
Looks like Tomcat grafted the changeset with the wrong bug number in it still.
https://hg.mozilla.org/releases/mozilla-beta/rev/6ede2ff230fb
This also appears to be causing SM(arm64) test failures. I've retriggered the job to be sure.
https://treeherder.mozilla.org/logviewer.html#?job_id=670128&repo=mozilla-beta
Flags: needinfo?(cbook) → needinfo?(steve)
Comment 18•9 years ago
|
||
Looks like SM(arm64) only recently started running on mozilla-beta and has been broken from the start. I think you can disregard that for now.
Flags: needinfo?(steve)
You need to log in
before you can comment on or make changes to this bug.
Description
•