[aarch64-port-dev ] RFR: 8179933: aarch64: incorrect match rule for immL_63 and immL_255

[Andrew Dinn]

On 12/05/17 08:24, Zhongwei Yao wrote:
> And I find immL_63's modification in the previous patch is incorrect. The
> original immL_63 definition is correct. I renamed immL_63 to the proper
> name: immI_63 in the updated patch.
> 
> I've uploaded to the new webrev at:
> http://cr.openjdk.java.net/~njian/8179933/webrev.01/
> ("hg export" looks better than the diff in webrev).
> 
> And here is my test case for "SubExtL_uxtb_and" instruction. Case for
> "AddExtL_uxtb_and" is similar.
> 
> java case:
>     public static long subAnd255() {
>            long acc1 = 0;
>            for (long i = 0; i < 10000; i++) {
>              acc1 -= (i & 255);
>            }
>            return acc1;
>     }
> 
> orignal generated code:
>   0xa0: add x19, x19, #0x1
>   0xa4: sub x20, x20, x11
>   0xa8: ldr wzr, [x10]
>   0xac: and x11, x19, #0xff
>   0xb0: mov x8, #0x2710
>   0xb4: cmp x19, x8
>   0xb8: b.lt 0xa0
> 
> generated code after applying this patch:
>   0x20: sub x20, x20, w19, uxtb
>   0x24: add x19, x19, #0x1
>   0x28: ldr wzr, [x10]
>   0x2c: mov x8, #0x2710
>   0x30: cmp x19, x8
>   0x34: b.lt 0x20

Yes, the change in the definition of immL_255 is correct and so is the
change in the name of immL_63 to immI_63.

n.b. immL_255 is used to type the mask operand to an AndL which is of
type long. So, its definition needs to match a long constant.

immL_63 was being used to type the shiftcount operands to RShiftL and
URShiftL which, of course, are ints. Since its definition already
matches an int renaming to immI_63 is correct.

The patch looks good to me.

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander