[jdk16] RFR: 8260585: AArch64: Wrong code generated for shifting right and accumulating four unsigned short integers [v3]

[dongbo (E)]

On 2021/2/1 20:32, Andrew Haley wrote:
> On 2/1/21 8:11 AM, dongbo (E) wrote:
>> The tests passed with the newest version and still can catch the typo.
>> Also tested a few of injected errors, the tests failed as expected.
> One oddity has come up.
>
> I'm running compiler/c2/TestShiftRightAndAccumulate
> and the generated code I see for compiler.c2.TestShiftRightAndAccumulate::test_shorts
>
> No vector instructions here. As far as I can see vectors are never used
> for jshort, just for jchar. All very strange, and probably not your fault,
> but since I'm looking I had to mention it.
>
> The other weird thing is that {u,s}sra is never generated with the .8B form.

I guess the `usra` is not generated for `byte` and `short` because:
```
src/hotspot/share/opto/vectornode.cpp, line 182:
   case Op_URShiftI:
     switch (bt) {
     case T_BOOLEAN:return Op_URShiftVB;
     case T_CHAR:   return Op_URShiftVS;
     case T_BYTE:
     case T_SHORT:  return 0; // Vector logical right shift for signed short
                              // values produces incorrect Java result for
                              // negative data because java code should 
convert
                              // a short value into int value with sign
                              // extension before a shift.
     case T_INT:    return Op_URShiftVI;
     default:       ShouldNotReachHere(); return 0;
     }
```

For `byte` and `short`, we can have `ssra` for the code below:

```

         for (int i = 0; i < count; i++) {
             shortsC[i] = (short) (shortsA[i] + (shortsB[i] >> 5));
             shortsD[i] = (short) (shortsA[i] + shortsB[i]);
         }

```

I updated the tests to use this code, although I don't know why the 
similar pattern of `char` does not works for `short`.

For `.8B` form, we only have `sshr + add`:

```

           │   0x0000ffff90085664:   sshr        v16.8b, v16.8b, #1
           │   0x0000ffff90085668:   add v16.8b, v16.8b, v17.8b

```

According to the current implementation, it is strange that they are not 
combined into a `ssra`:

```

AddVB:    match(Set dst (AddVB src1 src2))
RShiftVB: match(Set dst (RShiftVB src (RShiftCntV shift)))
SSRA_VB: match(Set dst (AddVB dst (RShiftVB src (RShiftCntV shift))))

```

I think we need further investigate the last two issues.