On Fri, 7 Mar 2025 12:25:51 GMT, Galder Zamarreño <gal...@openjdk.org> wrote:
>> @galderz Thanks for the summary of regressions! Yes, there are plenty of >> speedups, I assume primarily because of `Long.min/max` vectorization, but >> possibly also because the operation can now "float" out of a loop for >> example. >> >> All your Regressions 1-3 are cases with "extreme" probabilitiy (close to >> 100% / 0%), you listed none else. That matches with my intuition, that >> branching code is usually better than cmove in extreme probability cases. >> >> As for possible solutions. In all Regression 1-3 cases, it seems the issue >> is scalar cmove. So actually in all cases a possible solution is using >> branching code (i.e. `cmp+mov`). So to me, these are the follow-up RFE's: >> - Detect "extreme" probability scalar cmove, and replace them with branching >> code. This should take care of all regressions here. This one has high >> priority, as it fixes the regression caused by this patch here. But it would >> also help to improve performance for the `Integer.min/max` cases, which have >> the same issue. >> - Additional performance improvement: make SuperWord recognize more cases as >> profitble (see Regression 1). Optional. >> - Additional performance improvement: extend backend capabilities for >> vectorization (see Regression 2 + 3). Optional. >> >> Does that make sense, or am I missing something? > >> As for possible solutions. In all Regression 1-3 cases, it seems the issue >> is scalar cmove. So actually in all cases a possible solution is using >> branching code (i.e. `cmp+mov`). So to me, these are the follow-up RFE's: >> >> * Detect "extreme" probability scalar cmove, and replace them with branching >> code. This should take care of all regressions here. This one has high >> priority, as it fixes the regression caused by this patch here. But it would >> also help to improve performance for the `Integer.min/max` cases, which have >> the same issue. > > I've created [JDK-8351409](https://bugs.openjdk.org/browse/JDK-8351409) to > address this. @galderz Excellent. Testing looks all good on our side. Yes I think what you saw was unrelated. @rwestrel Could give this a last quick scan and then I think you can integrate :) ------------- PR Comment: https://git.openjdk.org/jdk/pull/20098#issuecomment-2706434983
