[11u] RFR 8206895: aarch64: rework error-prone cmp instuction

[Andrew Haley]

.On 3/23/21 6:29 PM, Hohensee, Paul wrote:
> I'd say this patch is worth doing on its own, since it catches actual argument size mismatches that would otherwise be silently accepted. Doesn't seem super intrusive either: all it does is add formal argument casts to "u1", and replace some cmp instructions with subs. Effectively syntax-only changes, so low risk.

Hmm, interesting. But it's *not* syntax only, as I mention later.

OK, but it's a lot of churn in an old maintenance release,
especially where there's no actual bug. Isn't churn a bad thing
of itself?

> I don't think we've been backporting all of the aarch64 back end work to 11u, just some of it, and we've (well, I've, so far) done the equivalent x64 work where needed too.

So what is the threshold for a backport? I'd think it should be
a significant user-visible bug (or performance enhancement.)
There are other possible criteria, such as misleading and error-
prone code, which this perhaps qualifies for. It's still almost
a thousand lines of patch, though.

And this really is error-prone code:

-    cmp(rscratch1, is_virtual ? DataLayout::virtual_call_type_data_tag : DataLayout::call_type_data_tag);
+    cmp(rscratch1, u1(is_virtual ? DataLayout::virtual_call_type_data_tag : DataLayout::call_type_data_tag));

because it silently truncates its argument.

It's what checked_cast was invented for,

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