[aarch64-port-dev ] RFR(S): 8239914: AArch64: Combine add/sub with comparison against zero

[Andrew Haley]

On 3/4/20 3:52 AM, Nick Gasson wrote:
>
> On 03/03/20 18:12 pm, Pengfei Li wrote:
>> Thanks for reporting this. I just tested the program and found the
>> exception also exists with AArch64 JDK master build (without my
>> patch).
>
> I couldn't reproduce this on the latest jdk/jdk, only with your patch. I
> think the problem is with the cmp*_sub_zero match rules. (a - b) > 0 !==
> a > b if a - b overflows because Java requires the result to wrap around
> which changes the sign.

Indeed. We need to think carefully about the way we review (and,
indeed, write) AArch64 patches. This small one might well have got
through to production without its fatal flaw being noticed. In this
case, though, the review process worked.

Unit tests are good, but we need to make sure that they test the
corner cases. Every arithmetic instruction can overflow.

Note that this is absolutely not a criticism of Pengfei; we all make
mistakes.

-- 
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