[aarch64-port-dev ] RFR(S): 8241911: AArch64: Fix a potential register clash issue in reduce_add2I

[Yang Zhang]

Hi Andrew

>instruct reduce_add4I(iRegINoSp dst, iRegIorL2I i_src, vecX v_src, vecX v_tmp, iRegINoSp i_tmp) %{

Besides reduce_add4I, other reduction operations (reduce_mul4I, reduce_max4F, etc) also have such issues. How about creating another JBS and patch to fix this issue? 

-----Original Message-----
From: Andrew Haley <aph at redhat.com> 
Sent: Thursday, April 9, 2020 5:42 PM
To: Yang Zhang <Yang.Zhang at arm.com>; hotspot-compiler-dev at openjdk.java.net; aarch64-port-dev at openjdk.java.net
Cc: nd <nd at arm.com>
Subject: Re: [aarch64-port-dev ] RFR(S): 8241911: AArch64: Fix a potential register clash issue in reduce_add2I

On 4/9/20 7:43 AM, Yang Zhang wrote:
> Hi
>
> Update the patch a little. Could you please help to review it?
> http://cr.openjdk.java.net/~yzhang/8241911/webrev.01/

I've been trying to figure out why this code is so difficult to understand. I think it's because names like tmp1 and src1 are used regardless of what kind of thing tmp1 is.

I suggest something like

instruct reduce_add4I(iRegINoSp dst, iRegIorL2I i_src, vecX v_src, vecX v_tmp, iRegINoSp i_tmp) %{
  match(Set dst (AddReductionVI i_src v_src));
  ins_cost(INSN_COST);
  effect(TEMP v_tmp, TEMP i_tmp);
  format %{ "addv  $v_tmp, T4S, $v_src\n\t"
            "umov  $i_tmp, $v_tmp, S, 0\n\t"
            "addw  $dst, $i_tmp, $i_src\t# add reduction4I"
  %}
  ins_encode %{
    __ addv(as_FloatRegister($v_tmp$$reg), __ T4S,
            as_FloatRegister($v_src$$reg));
    __ umov($i_tmp$$Register, as_FloatRegister($v_tmp$$reg), __ S, 0);
    __ addw($dst$$Register, $i_tmp$$Register, $i_src$$Register);
  %}
  ins_pipe(pipe_class_default);
%}

I think this makes the intent much clearer. Thanks.

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