[PATCH] Bug fix in TemplateTable::iop2 (JDK9 aarch32)

[Andrey Petushkov]

Dear Xiang,

Just in case, internally we made the same change and believe it’s correct

Thanks,
Andrey

> On 29 Dec 2015, at 16:21, Xiang Yuan <xiang.yuan at linaro.org> wrote:
> 
> Hi, All:
>  Our team fix a bug in JDK9 aarch32 template interpreter, the detail is
> described below:
> 
> *Test Case:*
> /***************code start***************/
> public class Iop2 {
>         public static void main(String args[]) {
>                   int shift_bits = 36; //0x24, 0b00100100
>                   int data = 0x87654321;
> 
>                   System.out.println(Integer.toHexString(data <<
> shift_bits));
>                   System.out.println(Integer.toHexString(data >>
> shift_bits));
>                   System.out.println(Integer.toHexString(data >>>
> shift_bits));
>         }
> }
> /***************code end****************/
> 
> The result of Openjdk9(aarch32):
> 0
> ffffffff
> 0
> 
> The correct result:
> 76543210
> f8765432
> 8765432
> 
> *Bug Description:*
>    This test case uses Java bytecodes ishl, ishr and iushr. To interpret
> them, aarch32 instructions lsl, asr and lsr are used directly in the
> procedure “void TemplateTable::iop2(Operation op) in
> ./hotspot/src/cpu/aarch32/vm/templateTable_aarch32.cpp”.
> 
> /***************code start***************/
>       case shl  : __ lsl(r0, r1, r0);  break;
>       case shr  : __ asr(r0, r1, r0);  break;
>       case ushr : __ lsr(r0, r1, r0);  break;
> /***************code end****************/
> 
>    The r1 is the int value need to be shifted, and the r0 is the shift
> bits.
> 
>    The JVM Specification requires that for these 3 bytecodes, they pop two
> int values from the expression stack as value2 and value1, and then they
> shift value1 by the low 5 bits of value2. However, these aarch32
> instructions shift
> value1 by the low 8 bits of value2. And the semantics of aarch32
> instructions don’t match the Java bytecodes. And therefore, using aarch32
> instruction to interpret these Java bytecodes directly will get a wrong
> result.
> 
> *Bug Solution:*
>         Clear the high 27 bits of value2 before shifting can fix this bug.
> /***************code start***************/
>       case shl  : __ andr(r0, r0, 31); __ lsl(r0, r1, r0);  break;
>       case shr  : __ andr(r0, r0, 31); __ asr(r0, r1, r0);  break;
>       case ushr : __ andr(r0, r0, 31); __ lsr(r0, r1, r0);  break;
> /***************code end****************/
> 
> *Bug Patch:*
> /***************patch start***************/
> --- a/src/cpu/aarch32/vm/templateTable_aarch32.cpp Wed Dec 23 12:21:32 2015
> +0000
> +++ b/src/cpu/aarch32/vm/templateTable_aarch32.cpp       Mon Dec 28
> 17:12:30 2015 +0800
> @@ -1275,9 +1275,9 @@
>   case _and : __ andr(r0, r1, r0); break;
>   case _or  : __ orr(r0, r1, r0);  break;
>   case _xor : __ eor(r0, r1, r0);  break;
> -  case shl  : __ lsl(r0, r1, r0);  break;
> -  case shr  : __ asr(r0, r1, r0);  break;
> -  case ushr : __ lsr(r0, r1, r0);  break;
> +  case shl  : __ andr(r0, r0, 31); __ lsl(r0, r1, r0);  break;
> +  case shr  : __ andr(r0, r0, 31); __ asr(r0, r1, r0);  break;
> + case ushr : __ andr(r0, r0, 31); __ lsr(r0, r1, r0);  break;
>   default   : ShouldNotReachHere();
>   }
> }
> /***************patch end****************/
> 
>  And the attachment are the test case and patch.
> 
>  Best regards!
> <Iop2.diff>