[arm64] Fix jssp based spill slot accesses in Crankshaft

[arm64] Fix jssp based spill slot accesses in Crankshaft

Committed: https://crrev.com/102e3e87e7bb8a647893149efa31bc2e771c7eb9
Cr-Commit-Position: refs/heads/master@{#31304}

[Jakob Kummerow, 2015-10-15 12:02:01]

jkummerow@chromium.org changed reviewers:
+ ulan@chromium.org

[Jakob Kummerow, 2015-10-15 12:02:01]

This has been broken since https://codereview.chromium.org/1038363002/ because
jssp tracking didn't take LDrop instructions into account, so we would access
the wrong stack slots.

As long as we have that mechanism, we should fix it, so please take a look.

That said, given that tracking stack pointer movements is obviously brittle, I
wonder whether we want to have this mechanism at all. What would be the impact
of removing it (reverting the CL mentioned above), and always using the fp
instead?

[jbramley, 2015-10-15 12:11:01]

On 2015/10/15 12:02:01, Jakob wrote:
> That said, given that tracking stack pointer movements is obviously brittle, I
> wonder whether we want to have this mechanism at all. What would be the impact
> of removing it (reverting the CL mentioned above), and always using the fp
> instead?

In ARM64, loads and stores have a much greater positive than negative offset
range, so it can be very beneficial to use jssp rather than fp. In general, code
like "mov x16, -fpoffset; ldr x0, [fp, x16]" becomes "ldr x0, [jssp, spoffset]".

The _original_ implementation claimed to get a code-size improvement, but not
much difference on benchmarks, though that was some time ago:
https://codereview.chromium.org/262533006

Also worth noting is that I'm currently trying to do something similar for
TurboFan: https://codereview.chromium.org/1376173003/
In that case, the performance benefit is very clear:

"Notable benchmark results include lua_binarytrees, which improves by about 7.5%
on A57 and 5% on A53. Several other asm.js benchmarks gain 2-4%."

[ulan, 2015-10-15 12:32:30]

The fix and test LGTM!

About having the feature or not: I personally would prefer simplicity and
similarity with other ports. This question deserves separate discussion in the
compiler team.

[Jakob Kummerow, 2015-10-15 12:33:54]

The CQ bit was checked by jkummerow@chromium.org

[commit-bot: I haz the power, 2015-10-15 12:34:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1401703003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1401703003/1

[commit-bot: I haz the power, 2015-10-15 13:33:58]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-10-15 13:34:25]

Patchset 1 (id:??) landed as
https://crrev.com/102e3e87e7bb8a647893149efa31bc2e771c7eb9
Cr-Commit-Position: refs/heads/master@{#31304}