[arm64] Address full-codegen issues with pools.

[arm64] Address full-codegen issues with pools.

Inline SMI checks in ICs are performed with a TBZ/TBNZ instruction, which has a
32 kB range. To allow patching the SMI check, the location of the TBZ/TBNZ
instruction is stored after the call to the IC using a MOVZ instruction, in
particular using 11 bits of the immediate (so the number of instructions
between the inline data and the SMI check must be encodable in 11 bits).

To make sure we do not exceed these ranges, we need to block pool emission
between the check, the patch info, and the label the check branches to.

BUG=

Review-Url: https://codereview.chromium.org/2917403002
Cr-Commit-Position: refs/heads/master@{#45735}
Committed: https://chromium.googlesource.com/v8/v8/+/c7fa0bf0e0173ddf814b19742073ce3fc63b6e20

[georgia.kouveli, 2017-06-05 14:36:27]

georgia.kouveli@arm.com changed reviewers:
+ bmeurer@chromium.org, jarin@chromium.org, mstarzinger@chromium.org

[Michael Starzinger, 2017-06-06 12:14:37]

LGTM. Do you happen to have a test around that triggers this which we could add?
If not or in case it's super flaky or long-running, then don't bother, just in
case you have one handy. :)

[georgia.kouveli, 2017-06-06 13:43:13]

On 2017/06/06 12:14:37, Michael Starzinger wrote:
> LGTM. Do you happen to have a test around that triggers this which we could
add?
> If not or in case it's super flaky or long-running, then don't bother, just in
> case you have one handy. :)

I don't have a test case - we spotted this issue with modified
kCheckConstPoolInterval, kApproxMaxDistToConstPool and kApproxMaxPoolEntryCount
parameters, which would trigger pool emission at different points (and allow for
larger pools). Modifying those parameters (to values that should still work)
made some of the existing tests fail.

We could hit the issue with the current values of the parameters, though, with a
combination of a large pool (the maximum size would be 4kB currently, excluding
the pool header, but it could grow if more entries are added while the pool is
blocked) and a large number of veneers that are forced to be emitted because of
the pool. It's probably a bit difficult to hit the right number of branches and
pool entries, but does not seem to be impossible.

[Michael Starzinger, 2017-06-06 14:34:59]

On 2017/06/06 13:43:13, georgia.kouveli wrote:
> On 2017/06/06 12:14:37, Michael Starzinger wrote:
> > LGTM. Do you happen to have a test around that triggers this which we could
> add?
> > If not or in case it's super flaky or long-running, then don't bother, just
in
> > case you have one handy. :)
> 
> I don't have a test case - we spotted this issue with modified
> kCheckConstPoolInterval, kApproxMaxDistToConstPool and
kApproxMaxPoolEntryCount
> parameters, which would trigger pool emission at different points (and allow
for
> larger pools). Modifying those parameters (to values that should still work)
> made some of the existing tests fail.
> 
> We could hit the issue with the current values of the parameters, though, with
a
> combination of a large pool (the maximum size would be 4kB currently,
excluding
> the pool header, but it could grow if more entries are added while the pool is
> blocked) and a large number of veneers that are forced to be emitted because
of
> the pool. It's probably a bit difficult to hit the right number of branches
and
> pool entries, but does not seem to be impossible.

Ack. Yeah, makes sense. Totally fine without a test then. Thanks! Still LGTM.

[georgia.kouveli, 2017-06-06 14:35:24]

The CQ bit was checked by georgia.kouveli@arm.com

[commit-bot: I haz the power, 2017-06-06 14:35:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-06 15:02:57]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496759724285690, "parent_rev":
"5005faed5cd4890dd2eb28384f09e68b10d07df2", "commit_rev":
"c7fa0bf0e0173ddf814b19742073ce3fc63b6e20"}

[commit-bot: I haz the power, 2017-06-06 15:03:04]

Description was changed from

==========
[arm64] Address full-codegen issues with pools.

Inline SMI checks in ICs are performed with a TBZ/TBNZ instruction, which has a
32 kB range. To allow patching the SMI check, the location of the TBZ/TBNZ
instruction is stored after the call to the IC using a MOVZ instruction, in
particular using 11 bits of the immediate (so the number of instructions
between the inline data and the SMI check must be encodable in 11 bits).

To make sure we do not exceed these ranges, we need to block pool emission
between the check, the patch info, and the label the check branches to.

BUG=
==========

to

==========
[arm64] Address full-codegen issues with pools.

Inline SMI checks in ICs are performed with a TBZ/TBNZ instruction, which has a
32 kB range. To allow patching the SMI check, the location of the TBZ/TBNZ
instruction is stored after the call to the IC using a MOVZ instruction, in
particular using 11 bits of the immediate (so the number of instructions
between the inline data and the SMI check must be encodable in 11 bits).

To make sure we do not exceed these ranges, we need to block pool emission
between the check, the patch info, and the label the check branches to.

BUG=

Review-Url: https://codereview.chromium.org/2917403002
Cr-Commit-Position: refs/heads/master@{#45735}
Committed:
https://chromium.googlesource.com/v8/v8/+/c7fa0bf0e0173ddf814b19742073ce3fc63...
==========

[commit-bot: I haz the power, 2017-06-06 15:03:06]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/c7fa0bf0e0173ddf814b19742073ce3fc63...