RFR(S): 8215792: AArch64: String.indexOf generates incorrect result

Dmitrij Pochepko dmitrij.pochepko at bell-sw.com
Wed Jan 9 14:50:54 UTC 2019


Hi all,


here is my version of this patch consisting of single "sub" instruction 
(I haven't changed test): 
http://cr.openjdk.java.net/~dpochepk/8215792/webrev.01/

cnt2 is a counter for characters yet to be checked. So, instead of 
checking all characters in source string for first character match 
(which was initial reason for this bug), now it check (str2len - str1len 
+ 1).


Actually I think this "sub" instruction was initially lost while working 
on this instrinsic and moving this instruction between this block 
(generate_string_indexof_linear) and caller code. Regular tests couldn't 
catch this problem.


I run some testing to ensure regular usecases are not affected and it 
seems fine. Affected testcase and your test pass as well.


btw: now this code is even faster, because less characters will be 
loaded and checked


Thanks,

Dmitrij

On 04/01/2019 3:52 PM, Dmitrij Pochepko wrote:
> Sure.
>
> I could miss something, so, need to try it. I'll send webrev with 
> patch once it's done.
>
>
> Thanks,
>
> Dmitrij
>
>
> On 04.01.2019 14:04, Pengfei Li (Arm Technology China) wrote:
>> Hi Dmitrij,
>>
>> Thanks a lot for your reply.
>>
>>> since cnt2 is used as counter, wouldn't it be easier and shorter 
>>> just to substract cnt1 from cnt2 at the beginning of this code. 
>>> Total (cnt2 - cnt1 +1) combinations must be checked. That is why 
>>> first sustraction is by (wordSize/str2_chr_size - 1).
>>> Then whole fix will be probably just 1 line at the beginning: 
>>> sub(cnt2, cnt2, cnt1);
>> I don't think the whole fix could be as easy as "sub(cnt2, cnt2, 
>> cnt1)" because cnt2 is the counter which counts number of bytes not 
>> processed. It could be different from the number of bytes after 
>> current first-character-match index.
>>
>> But this is just my thought. Perhaps I didn't understand your idea 
>> and code thoroughly. So could you post your shorter fix and let's 
>> test if it's right?
>>
>> -- 
>> Thanks,
>> Pengfei
>>
>


More information about the hotspot-compiler-dev mailing list