On Mon, 20 Jan 2025 08:00:52 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> @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 š @eme64 Fair points. I'll provide a detailed summary with some final numbers after FOSDEM. ------------- PR Comment: https://git.openjdk.org/jdk/pull/20098#issuecomment-2617968437
