RFR(S): 8235762: JVM crash in SWPointer during C2 compilation

[Tobias Hartmann]

Hi Felix,

looks reasonable to me. Thanks for updating.

Another review would be good.

Best regards,
Tobias

On 10.01.20 02:13, Yangfei (Felix) wrote:
> Hi Tobias,
> 
>   Updated webrev: http://cr.openjdk.java.net/~fyang/8235762/webrev.01/ 
>   This also updated copyright years of the newly-added test case.  
>   Still test OK with the latest repo.  Is this one better?  
> 
> Thanks,
> Felix
> 
>>
>> Hi Felix,
>>
>> On 16.12.19 11:14, Yangfei (Felix) wrote:
>>> -- OK.  I will add one assertion immediately before breaking the loop.
>>
>> Thanks.
>>
>>>> - Why do you need max_idx? Isn't is the case that if
>>>> find_align_to_ref returns NULL, there aren't any comparable memory
>>>> operations left and it essentially doesn't matter which one you chose?
>>>
>>> -- I was considering minimizing the performance impact of the patch here.
>>>   best_align_to_mem_ref is used in SuperWord::align_initial_loop_index for
>> adjusting the pre-loop limit.
>>>   For the test case in the webrev, best_align_to_mem_ref was chosen from
>> node 470 (StoreB) and node 431 (StoreL).
>>>   The vector width for these two memory operations are different on
>> aarch64 platform: vw = 16 bytes for node 431 and 2 bytes for node 470.
>>>   SuperWord::align_initial_loop_index will emit different code sequences for
>> the test case.
>>>   The max_idx tells us which memory operation has the biggest vector size.
>>
>> Okay, thanks for the details. Could you add a comment that explains this?
>>
>>>> Also, is it guaranteed that max_idx is always initialized in
>> SuperWord::find_align_to_ref?
>>>
>>> -- Yes, I think so.
>>>   When memops is not empty and the memory ops in memops are not
>> comparable, find_align_to_ref will always sets its max_idx.
>>
>> Okay.
>>
>> Thanks,
>> Tobias