[aarch64-port-dev ] [aarch64-port-dev] [10] RFR: 8184049 : Matching rule for ubfiz

[Daniel Stewart]

So, given that Felix has no issues, is this good to push?

On Wed, Aug 23, 2017 at 9:47 AM, Felix Yang <felix.yang at linaro.org> wrote:

> Hi,
>
>     As I mentioned in http://mail.openjdk.java.net/pipermail/aarch64-port-
> dev/2017-August/004767.html, your modification looks good to me.
>     I posted the GCC's way of doing the matching condition check just for
> a reference.  As I see it, your modification is equal in functionality.
>
> Thanks,
> Felix
>
> On 23 August 2017 at 20:12, Daniel Stewart <daniel.stewart at linaro.org>
> wrote:
>
>> Felix,
>>
>> I'm not sure I understand the check here and exactly what you are
>> proposing. It appears to me that this check is for an arithmetic right
>> shift, not a left shift, which is what we have my proposed solution. I
>> could be wrong as I am not familiar with GCC, but I can't make sense of the
>> check if the shift is a left shift. (Perhaps GCC is using the shift value
>> as if it were encoded for a ubfm instruction?) Could you explain a bit more
>> about what exactly you'd like to see?
>>
>> Thanks,
>> Daniel
>>
>> On Tue, Aug 1, 2017 at 11:10 AM, Felix Yang <felix.yang at linaro.org>
>> wrote:
>>
>>> LGTM, but I am not an official reviewer.
>>>
>>> Here is how the latest GCC compiler do the check, which is more readable
>>> to me:
>>>
>>> /* Return true if the mask and a shift amount from an RTX of the form
>>>    (x << SHFT_AMNT) & MASK are valid to combine into a UBFIZ instruction
>>> of
>>>    mode MODE.  See the *andim_ashift<mode>_bfiz pattern.  */
>>>
>>> bool
>>> aarch64_mask_and_shift_for_ubfiz_p (machine_mode mode, rtx mask, rtx
>>> shft_amnt)
>>> {
>>>   return CONST_INT_P (mask) && CONST_INT_P (shft_amnt)
>>> && INTVAL (shft_amnt) < GET_MODE_BITSIZE (mode)
>>> && exact_log2 ((INTVAL (mask) >> INTVAL (shft_amnt)) + 1) >= 0
>>> && (INTVAL (mask) & ((1 << INTVAL (shft_amnt)) - 1)) == 0;
>>> }
>>>
>>> Thanks,
>>> Felix
>>>
>>> On 31 July 2017 at 20:22, Daniel Stewart <daniel.stewart at linaro.org>
>>> wrote:
>>>
>>>> Updated webrev to address Felix's issue regarding the format.
>>>>
>>>> Please review and let me know of any changes/fixes.
>>>>
>>>> http://cr.openjdk.java.net/~njian/8184049/webrev.02/
>>>>
>>>> Daniel
>>>>
>>>> On Fri, Jul 28, 2017 at 1:37 PM, Andrew Haley <aph at redhat.com> wrote:
>>>>
>>>>> On 27/07/17 21:49, Daniel Stewart wrote:
>>>>> > public void run(String [] args) {
>>>>> >         long sum = this.sum | (1 << 27);
>>>>> >         int n = (int)sum;
>>>>> >         n = xorshift32(n);
>>>>> >         for (int i = 0; i < 1000; i++) {
>>>>> >             //System.out.println((int)Math.random());
>>>>> >             n += testI(n);
>>>>> >             n += testI2(n);
>>>>> >             n += testI3(n);
>>>>> >             n += testI4(n);
>>>>> >             n += testI5(n);
>>>>> >         }
>>>>> >         long n1 = (long)n;
>>>>> >         for (int i = 0; i < 1000; i++) {
>>>>> >             n1 += testL(n1);
>>>>> >             n1 += testL2(n1);
>>>>> >             n1 += testL3(n1);
>>>>> >             n1 += testL4(n1);
>>>>> >             n1 += testConv(n1);
>>>>> >         }
>>>>> >         short n2 = (short)n;
>>>>> >         for (int i = 0; i < 1000; i++) {
>>>>> >             n2 += testConv2((short)n2);
>>>>> >         }
>>>>> >         n1 += n2;
>>>>> >         for (int i = 0; i< 1000; i++) {
>>>>> >             n1 += testConv3(n);
>>>>>
>>>>> This loop doesn't test the intrinsic.  I added
>>>>>
>>>>>                n = xorshift32(n);
>>>>>
>>>>> here.
>>>>>
>>>>> >         }
>>>>> >         this.sum += sum ^ n1;
>>>>>
>>>>>
>>>>> --
>>>>> Andrew Haley
>>>>> Java Platform Lead Engineer
>>>>> Red Hat UK Ltd. <https://www.redhat.com>
>>>>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Daniel Stewart
>>>>
>>>
>>>
>>
>>
>> --
>> Daniel Stewart
>>
>
>


-- 
Daniel Stewart