[aarch64-port-dev ] RFR(S/M): 8247766: [aarch64] guarantee(val < (1U << nbits)) failed: Field too big for insn

[Andrew Haley]

Hi,

On 13/07/2020 16:33, Patric Hedlin wrote:
>
> On 2020-07-09 16:26, Andrew Haley wrote:
>> On 07/07/2020 12:17, Patric Hedlin wrote:
>>> C1 code generation for reading and writing stack-slots does not handle
>>> large immediate offsets on aarch64. This patch will ensure that
>>> immediate offsets are admissible for base+(immediate)offset encoding or,
>>> if this is not the case, will enforce an explicit address calculation to
>>> a scratch register. (Also correcting a small glitch in 9-bit signed
>>> immediate encoding check.)
>>>
>>> NOTE: Current patch includes (local) definitions of is_simm/9 and
>>> is_uimm/12, for review purpose only. With JDK-8248901 these will move to
>>> Assembler, and will not be included in the change-set.
>> Umm, OK. These functions seem too complicated: all you have to do is
>>
>>      int64_t chk = val >> (nbits - 1);                                                                                                   |(gdb)
>>      guarantee (chk == -1 || chk == 0, "Field too big for insn");                                                                        |#9  0x0000ffffbcab5c30 in Compilation::compile_method (this=0xffff80b7dde8)
> The 'guarantee' of course works poorly as a predicate and the 'chk'
> calculation is based on implementation-dependent behaviour.

Hmm. Signed >> does require a 2's complement C implementation, but we
assume many implementation-defined things in HotSpot. I know, if we
can remove such things perhaps we should, all other things being
equal.

>> but the AArch64 part of it looks fine.
> Having seen your second answer, I guess you had a change of
> hart. What a pity.

Of course I want this bug fixed, and I'm grateful for this patch.

However, we already have an equivalent overflow test in a couple of (a
few, probably) different places; and your patch adds another one. We
shouldn't be doing that.

I'm quite open to doing it in another way, thereby replacing the
existing logic, but not to duplicating code.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671