On Fri, 17 Jan 2025 17:41:47 GMT, Galder ZamarreƱo <gal...@openjdk.org> wrote:
>> @galderz I ran some testing on our side, feel free to ping me in 1-2 days >> for the results. > > @eme64 I've addressed all the comments. I've not run the `VectorReduction2` > for the reasons explained in the previous comment. Happy to add more details > to `MinMaxVector` if you feel it's necessary. @galderz Ah, right. I understand about the branch probability. Hmm, maybe we should eventually change the `VectorReduction2` benchmark, or just remove the `min/max` benchmark there completely, as it depends on the random input values. Ah, though we have a fixed `seed`, so rerunning the benchmark would at least have consistent branching characteristics. So then it could make sense to run the benchmark, we just don't know the probability. I mean I ran it before for the `in/float/double min/max`, and all of them see a solid speedup. So I would expect the same for `long`, it would be nice to at least see the numbers. You could extend your benchmark to `float / double` as well, to make it complete. But that could also be a follow-up RFE. >I would expect a similar thing to happen when it comes to asimd envs with max >vector size >= 32 (e.g. Graviton 3). Those will see vectorization occur and >improvements kick in at 100%. Other systems (e.g. Graviton 4) will see a >regression at 100%. This means that your work in >https://github.com/openjdk/jdk/pull/20098#discussion_r1901576209 to avoid the >max vector size limitation might become more important once my PR here goes in. So are you saying there are machines where we are now getting some regressions with your patch (2-element cases)? It would be nice to see the numbers summarized here. I'm losing the overview a little over the 50+ messages now š ------------- PR Comment: https://git.openjdk.org/jdk/pull/20098#issuecomment-2601678386
