1266366 - [ppc64] [ppc64le] 45.1.0 ESR build fails because class js::jit::MacroAssembler has no member named 'branch64'

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan Horak [:jhorak]]

We've hit following issue with Firefox 45.1.0 ESR build on ppc64 and ppc64le:

In file included from /builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/objdir/js/src/Unified_cpp_js_src13.cpp:2:0:
/builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/js/src/jit/IonCaches.cpp: In function 'void CheckDOMProxyExpandoDoesNotShadow(JSContext*, js::jit::MacroAssembler&, JSObject*, jsid, js::jit::Register, js::jit::Label*)':
/builddir/build/BUILD/firefox-45.1.0/firefox-45.1.0esr/js/src/jit/IonCaches.cpp:702:14: error: 'class js::jit::MacroAssembler' has no member named 'branch64'
         masm.branch64(Assembler::NotEqual,
              ^

It seems that it was cased by https://bugzilla.mozilla.org/show_bug.cgi?id=1246061 (this change http://hg.mozilla.org/mozilla-central/diff/967dcb05f347/js/src/jit/IonCaches.cpp#l703 ).

Firefox 45.0.2 ESR build under same conditions is fine.

Comment 1

[Nicolas B. Pierron [:nbp]]

This is a problem in the backported patch, which lacks definition of the branch64 function in the none macro assembler.  I will make a patch in a few minutes.

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: Add branch64 functions to the none-backend MacroAssembler.

Comment 3

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  Bug 1246061 caused a compilation issue on builds with disabled jit.
User impact if declined: Cannot compile.
Fix Landed on Version: N/A
Risk to taking this patch (and alternatives if risky): N/A
String or UUID changes made by this patch: N/A

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

Taking it for 45.2.0.
It might be in 45.1.0 if we find a driver for a second build.

This missed 46 RC2, if we do a RC3, we could take it but this is Liz's call.

Comment 5

[Jeff Walden [:Waldo]]

Bah, sorry for missing these.  Backport was a little beastly here because of significant trunk code improvements, and as the trunk patch didn't (because of that) have none-arch changes, I didn't remember I needed to make them explicitly in backports.

I also didn't make these explicit changes in the ESR38 backport.  Is it necessary to fix that, or is ESR38 sufficiently Johnny-come-lately that the absence there won't hurt you?

Comment 6

[Jan Horak [:jhorak]]

Thanks guys for swift reaction!

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/797112ba4b5b

Comment 8

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

I missed this on Friday; we did an rc3 but i don't think this is in it.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> I missed this on Friday; we did an rc3 but i don't think this is in it.

It's not, fwiw.

Comment 10

[Mike Hommey [:glandium]]

(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> This is a problem in the backported patch, which lacks definition of the
> branch64 function in the none macro assembler.  I will make a patch in a few
> minutes.

It doesn't look like the none macro assembler has a branch64 method in any branch (except esr45, now)

Comment 11

[Nicolas B. Pierron [:nbp]]

(In reply to Mike Hommey [:glandium] from comment #10)
> It doesn't look like the none macro assembler has a branch64 method in any
> branch (except esr45, now)

Yes, this issue is caused by the backported patches from Bug 1246061.  Newer branches are using the generic MacroAssembler, with the "PER_ARCH" macro which replaces the need for the none macro assembler.

The status flags are correct.

Comment 12

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

Do we need this on 46.0.1 desktop (planned for this coming week)  
Or are you aiming it at the next esr?

Comment 13

[Mike Hommey [:glandium]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Do we need this on 46.0.1 desktop (planned for this coming week)  
> Or are you aiming it at the next esr?

It would be nice if it were fixed in both next esr and next point release. (is there a planned esr along 46.0.1?)

Comment 14

[Nicolas B. Pierron [:nbp]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Do we need this on 46.0.1 desktop (planned for this coming week)  
> Or are you aiming it at the next esr?

As Sylvestre mention, this is a nice to have but not enough to make a new release on its own.

So, yes, we should take it.

Comment 15

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

Comment on attachment 8743851 [details] [diff] [review]
Add branch64 functions to the none-backend MacroAssembler.

Approved for m-r uplift so it will go into 46.0.1. Otherwise debian/redhat users can't build firefox

Comment 16

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

https://hg.mozilla.org/releases/mozilla-release/rev/5c6b2ef03b9b

Comment 17

[Till Schneidereit [:till]]

Fixed by landing on the release and esr branches.