RFR(S): 8215792: AArch64: String.indexOf generates incorrect result
Pengfei Li (Arm Technology China)
Pengfei.Li at arm.com
Tue Jan 22 10:32:00 UTC 2019
Hi Dmitrij,
I (not a reviewer) tested your single line fix and it looks ok to me.
Also I bump the priority of the JBS (https://bugs.openjdk.java.net/browse/JDK-8215792) to P2 and hope the fix could be in JDK 12. (The door of JDK 12 will be closed soon)
> Indeed it is hard to review complex algorithms. The Boyer-Moore comments
> you referenced were updated as part of the original webrev to describe
> changes in algorithm E, which is in macroAssembler_aarch64.cpp. I once
> asked to validate the level of comments with you during pow function review
> [3]. If this is the level of comments you find reasonable, I’ll be happy to
> improve it here and elsewhere to this level.
When I was trying to fix this bug, I found it pretty easy to get lost among branches in the code. And other engineers from Arm looking at the AArch64 intrinsics have the similar feeling. So I'd also strongly recommend you write more explanations in comments. In this String.indexOf(str) intrinsic, as there are a lot of paths for different length conditions, I have another suggestion of adding the conditions of path A to G you wrote in your last email into comments.
--
Thanks,
Pengfei
More information about the hotspot-compiler-dev
mailing list