1103108 - Reduce the number of call paths on arm & mips32

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Marty Rosenberg [:mjrosenb]]

Currently, there is CallIon, CallIonNoPush and CallIonHalfPush. Bug 1088316 is going to change Ion to use the hardware calling convention, at which point these don't make sense, and should be removed.

Comment 1

[Sean Stangl [:sstangl]]

It looks like removing these functions depends on changing the Ion ABI to align before the call is made, counting the possibly-pushed return address as part of the newly created frame.

Comment 2

[Luke Wagner [:luke]]

FWIW, I'm going to be removing the callJitNoPush from GenerateFFIIonExit in bug 1125561; it's kindof hilarious: we worked around the old terrible ARM convention in Odin and then, when that got fixed, CallJitNoPush had to work around our dependence on the old conversion.

Comment 3

[Nicolas B. Pierron [:nbp]]

We don't have to use ABI-compatible calls for Ion calls, to get rid of these multiple paths.
Because, there is one function which is never used (ma_callJit), and one used once (ma_callJitNoPush).

These functions were used at the time when we tried to push the return address before calling the callee, but now, all the callee are pushing their link registers, both on ARM and MIPS.

I have to clean-up these before refactoring all architectures Jit calls with Bug 1184965.

Comment 4

[Nicolas B. Pierron [:nbp]]

Attachment: part 1 - ARM: Remove unused ma_callJit.

Comment 5

[Nicolas B. Pierron [:nbp]]

Attachment: part 2 - MIPS32: Remove unused ma_callJit.

Comment 6

[Nicolas B. Pierron [:nbp]]

Attachment: part 3 - ARM: Replace single use ma_callJitNoPush by its implementation.

Comment 7

[Nicolas B. Pierron [:nbp]]

Attachment: part 4 - MIPS32: Remove unused ma_callJitNoPush.

Comment 8

[Nicolas B. Pierron [:nbp]]

ma_callJit has to be unused, because it is pushing extra space on the stack, which does not make sense any more as all our callee are now pushing their link registers on top of the stack, and not just write in back in the reserved space, as it used to be a while ago.

Comment 9

[Hannes Verschore [:h4writer]]

Comment on attachment 8646999 [details] [diff] [review]
part 1 - ARM: Remove unused ma_callJit.

Review of attachment 8646999 [details] [diff] [review]:
-----------------------------------------------------------------

Nice. Apparently dead code removal :D.

::: js/src/jit/arm/MacroAssembler-arm.cpp
@@ +1891,5 @@
>  
>  void
>  MacroAssemblerARMCompat::callJit(Register callee)
>  {
> +    MOZ_ASSERT((asMasm().framePushed() & 7) == 4);

what about foo % 8 = 4 instead of the & ? Seems more readable for me. Not sure if that is general. Feel free to ignore...

Comment 10

[Nicolas B. Pierron [:nbp]]

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

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f31396bf697
https://hg.mozilla.org/integration/mozilla-inbound/rev/afd4b1a461cc
https://hg.mozilla.org/integration/mozilla-inbound/rev/7047d316b864
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcf151980ac3

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0f31396bf697
https://hg.mozilla.org/mozilla-central/rev/afd4b1a461cc
https://hg.mozilla.org/mozilla-central/rev/7047d316b864
https://hg.mozilla.org/mozilla-central/rev/fcf151980ac3