993174 - Fix non-unified build error in Simulator-arm.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Dan Gohman [:sunfish]]

Attachment: decl-order.patch

Simulator-arm.cpp uses __aeabi_idivmod and __aeabi_uidivmod before it defines them. This appears to work in unified builds, because they are declared in other cpp files, but in the non-unified build, those aren't visible, so it doesn't build. The attached patch fixes this by moving the definitions of these functions to the top of Simulator-arm.cpp so that they're declared before being used.

Comment 1

[Dan Gohman [:sunfish]]

Comment on attachment 8402987 [details] [diff] [review]
decl-order.patch

Here's the patch you suggested on irc.

Comment 2

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6ba4478e9d2

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a6ba4478e9d2

Comment 4

[Douglas Crosher [:dougc]]

Noticed that this bug is causing a failure on try runs of the beta branch. It's only a small patch, and might it be useful to uplift it?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

That means it also affects being able to push B2G 1.4 to Try, so I would agree that we should probably take it. Please nominate :)

Comment 6

[Douglas Crosher [:dougc]]

Comment on attachment 8402987 [details] [diff] [review]
decl-order.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Bug 959597.


User impact if declined: 

None, but it causes a failure running a try build on the beta branch see:
https://tbpl.mozilla.org/php/getParsedLog.php?id=39898819&tree=Try


Testing completed (on m-c, etc.): 

Tested on m-c, and it fixed a failure on a try run on the beta branch.


Risk to taking this patch (and alternatives if risky): 

Low risk, the patch is to the ARM simulator used for testing and does not affect deployed code.


String or IDL/UUID changes made by this patch: n/a

Comment 7

[Jan de Mooij [:jandem]]

Note that this patch doesn't need approval AFAIK; Simulator-arm.cpp is NPOTB.

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8402987 [details] [diff] [review]
decl-order.patch

Go ahead with a=NPOTB in that case.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/46fdf6041673

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/46fdf6041673