Closed Bug 1208876 Opened 9 years ago Closed 9 years ago

add 64 bit operations to MacroAssembler-none


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox43 --- fixed
firefox44 --- fixed


(Reporter: stevensn, Assigned: stevensn)




(1 file, 1 obsolete file)

Patches in bug 774364 moved/added some 64 bit operation functions to the architecture MacroAssembler-XXX classes but missed MacroAssembler-none.
Add missing functions to avoid the compiler errors with no jit.
Attachment #8666494 - Flags: review?(sstangl)
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+
Passing by: wrong bug reference in the commit message.
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
Sorry, I mistyped the bug number in the commit comment.  This has been applied
Flags: needinfo?(steve)
Closed: 9 years ago
Resolution: --- → FIXED
Updating the comment in the patch to include the proper bug #
Attachment #8666494 - Attachment is obsolete: true
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 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 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?
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
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)
(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)
Oh of course. Thanks!
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+
Looks like Tomcat grafted the changeset with the wrong bug number in it still.

This also appears to be causing SM(arm64) test failures. I've retriggered the job to be sure.
Flags: needinfo?(cbook) → needinfo?(steve)
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.