On Fri, 30 May 2025 08:15:22 GMT, Xiaohong Gong <xg...@openjdk.org> wrote:
>>> @XiaohongGong Thanks for splitting this one out, and for investigating the >>> regressions here. >>> >>> Putting the permalink here, fixed to the current change (the link you >>> pasted will always refer to the newest, which may later on point to the >>> wrong line when lines above are inserted / deleted): >>> >>> https://github.com/openjdk/jdk/blob/7077535c0b0a6ea0a2a167f9135b1504a3d71fb3/src/hotspot/share/opto/loopnode.cpp#L1659-L1661 >>> >>> I wonder if we should just use `Node::uncast` there? But I'm quite unsure >>> about that. >> >> Sounds good to me. I will have a deep investigation for it. Thanks! >> >> >> >>> > Yes, I also observed such regression. >>> > It would be nice if you proactively mentioned regressions, so it does not >>> > have to be pointed out by reviewers. >>> >>> For me, it could be ok to fix it in a follow-up patch. I think we are too >>> close to RDP1 for JDK25 now anyway, and so we could push this patch here >>> into JDK26, and then we have enough time in JDK26 to investigate the >>> regression. Even better would be if we could do the other patch first, so >>> we never even encounter a regression. >> >> Sounds good to me. Thanks! > >> > @XiaohongGong Thanks for splitting this one out, and for investigating the >> > regressions here. >> > Putting the permalink here, fixed to the current change (the link you >> > pasted will always refer to the newest, which may later on point to the >> > wrong line when lines above are inserted / deleted): >> > https://github.com/openjdk/jdk/blob/7077535c0b0a6ea0a2a167f9135b1504a3d71fb3/src/hotspot/share/opto/loopnode.cpp#L1659-L1661 >> > >> > I wonder if we should just use `Node::uncast` there? But I'm quite unsure >> > about that. >> >> Sounds good to me. I will have a deep investigation for it. Thanks! > > Hi @eme64 @jatin-bhateja, I'v created a PR > https://github.com/openjdk/jdk/pull/25539 to fix this issue. With this > change, the performance regression can be fixed as well. Could you please > take a look at that change and help to run the test on different X86 > machines? Thanks a lot! > @XiaohongGong I reviewed #25539. Since it is a relatively simple patch, I > suggest that we integrate that one first, and come back to this here later. > Is that ok for you? That's fine to me. Thanks for your review! ------------- PR Comment: https://git.openjdk.org/jdk/pull/25138#issuecomment-2933082670
