[aarch64-port-dev ] RFR(S): 8215792: AArch64: String.indexOf generates incorrect result

[Dmitry Samersoff]

Dmitrij,

I'm OK with this one-line fix.

I would recommend to push the patch that fixes actual bug (webrev.01),
then proceed with the documentation (i.e. webrev.02) in a separate turn.

-Dmitry

On 09.01.2019 17:50, Dmitrij Pochepko wrote:
> 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
>>>
>>