[PING] RE: RFR(S): 8210152: Optimize integer divisible by power-of-2 check

[Tobias Hartmann]

Hi Pengfei,

sure, pushed:
http://hg.openjdk.java.net/jdk/jdk/rev/bc38c75eed57

Best regards,
Tobias

On 25.09.2018 12:13, Pengfei Li (Arm Technology China) wrote:
> Hi Tobias,
> 
> Thanks for your review comment. I've fixed the whitespaces and below is a new webrev.
> http://cr.openjdk.java.net/~njian/8210152/webrev.02/
> 
> Could you help push this patch since I don't have permissions to do that?
> 
> --
> Thanks,
> Pengfei
> 
> 
>> -----Original Message-----
>>
>> Hi Pengfei,
>>
>> this looks good to me but please fix the whitespacing before pushing:
>> "if( a == b )" -> "if (a == b)"
>> "method( param )" -> "method(param)"
>>
>> It's not consistently like that in old HotSpot code but we should at least fix it
>> in new code.
>>
>> Thanks,
>> Tobias
>>
>> On 25.09.2018 10:38, Pengfei Li (Arm Technology China) wrote:
>>> Hi Vladimir,
>>>
>>> I still didn't get other comments during the past week.
>>> Do you think it is ok to push this patch?
>>> http://cr.openjdk.java.net/~njian/8210152/webrev.01/
>>>
>>> --
>>> Thanks,
>>> Pengfei
>>>
>>>> -----Original Message-----
>>>>
>>>> Hi Reviewers,
>>>>
>>>> Is there any other comments, objections or suggestions on the new
>> webrev?
>>>> If no problems, could anyone help to push this commit?
>>>>
>>>> I look forward to your response.
>>>>
>>>> --
>>>> Thanks,
>>>> Pengfei
>>>>
>>>>> -----Original Message-----
>>>>>
>>>>> Looks good.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 9/12/18 2:50 AM, Pengfei Li (Arm Technology China) wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I've updated the patch based on Vladimir's comment. I added checks
>>>>>> for
>>>>> SubI on both branches of the diamond phi.
>>>>>> Also thanks Roland for the suggestion that supporting a Phi with 3
>>>>>> or more
>>>>> inputs. But I think the matching rule will be much more complex if
>>>>> we add this. And I'm not sure if there are any real case scenario
>>>>> which can benefit from this support. So I didn't add it in.
>>>>>>
>>>>>> New webrev: http://cr.openjdk.java.net/~njian/8210152/webrev.01/
>>>>>> I've run jtreg full test with the new patch and no new issues found.
>>>>>>
>>>>>> Please let me know if you have other comments or suggestions. If no
>>>>> further issues, I need your help to sponsor and push the patch.
>>>>>>
>>>>>> --
>>>>>> Thanks,
>>>>>> Pengfei
>>>>>>
>>>>>>