RFR: 8261022: Fix incorrect result of Math.abs() with char type [v2]

[Nils Eliasson]

On Fri, 5 Feb 2021 08:26:56 GMT, Pengfei Li <pli at openjdk.org> wrote:

>> Math.abs() with char type may return incorrect result after C2 superword optimization. It can be reproduced by below Java code and commands.
>> 
>> public class Bug {
>>     private static int SIZE = 60000;
>>     private static char[] a = new char[SIZE];
>>     private static char[] b = new char[SIZE];
>> 
>>     public static void main(String[] args) {
>>         for (int i = 0; i < SIZE; i++) {
>>             a[i] = b[i] = (char) i;
>>         }
>>         for (int i = 0; i < SIZE; i++) {
>>             a[i] = (char) Math.abs(a[i]);
>>         }
>>         for (int i = 0; i < SIZE; i++) {
>>             if (a[i] != b[i]) {
>>                 throw new RuntimeException("Broken!");
>>             }
>>         }
>>         System.out.println("OK");
>>     }
>> }
>> 
>> // $ java -Xint Bug
>> // OK
>> 
>> // $ java -Xcomp -XX:-TieredCompilation Bug
>> // Exception in thread "main" java.lang.RuntimeException: Broken!
>> //         at Bug.main(Bug.java:15)
>> 
>> In Java, 'char' is a 16-bit unsigned integer type and the abs() method should always return the value of its input. But with C2 vectorization, the sign bit of the 16-bit value is cleared because it's regarded as a signed value.
>> 
>> Root cause is that we get an imprecise vector element type for AbsINode from SuperWord::compute_vector_element_type(). In any Java arithmetic operation, operands of small integer types (boolean, byte, char & short) should be promoted to int first. As vector elements of small types don't have upper bits of int, for RShiftI or AbsI operations, the compiler has to know the precise signedness info of the 1st operand. These operations shouldn't be vectorized if the signedness info is imprecise.
>> 
>> In code SuperWord::compute_vector_element_type(), we have some special handling for right shift. It limited the vectorization of small integer right shift to operations only after loads. The reason is that in the C2 compiler, only LoadNode has precise signedness info of its value. When JDK-8222074 enabled abs vectorization, it didn't involve AbsI operation into the special handling and thus introduced this bug. This patch just does the fix at this point.
>> 
>> Tested hotspot::hotspot_all_no_apps, jdk::jdk_core and langtools::tier1, no new failure is found. Also created a new jtreg with this fix.
>
> Pengfei Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments

Looks good.

-------------

Marked as reviewed by neliasso (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2419