RFR(S): 8215792: AArch64: String.indexOf generates incorrect result
dmitrij.pochepko at bell-sw.com
Wed Jan 9 14:50:54 UTC 2019
here is my version of this patch consisting of single "sub" instruction
(I haven't changed test):
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
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
On 04/01/2019 3:52 PM, Dmitrij Pochepko wrote:
> I could miss something, so, need to try it. I'll send webrev with
> patch once it's done.
> 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?
More information about the hotspot-compiler-dev