1208876 - add 64 bit operations to MacroAssembler-none

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Steve Singer (:stevensn)]

Patches in bug 774364 moved/added some 64 bit operation functions to the architecture MacroAssembler-XXX classes but missed MacroAssembler-none.

Comment 1

[Steve Singer (:stevensn)]

Attachment: Add functions to MacroAssembler-none

Add missing functions to avoid the compiler errors with no jit.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

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!

Comment 3

[Steve Singer (:stevensn)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c78aa2af856

Comment 4

[Francesco Lodolo [:flod]]

Passing by: wrong bug reference in the commit message.

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[Steve Singer (:stevensn)]

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

Comment 7

[Steve Singer (:stevensn)]

Attachment: Add functions to MacroAssembler-none

Updating the comment in the patch to include the proper bug #

Comment 8

[Steve Singer (:stevensn)]

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

Comment 9

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none

Build issue, OK to uplift to aurora.

Comment 10

[Ian Stakenvicius]

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.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

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?

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I don't believe it has been uplifted to 43, likely because the target milestone was never set.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 14

[Jan de Mooij [:jandem]]

(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.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

Oh of course. Thanks!

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8669357 [details] [diff] [review]
Add functions to MacroAssembler-none

Not part of the build, ok to uplift to beta

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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.