This PR adds missing AtomicRMWKind::min/max cases which we would like to use for min/max reduction loop vectorizations.
Hi @aartbik, thank you for your reply. And agree, it would be nice to add testing. I was not sure how to do that for min/max reductions without proper changes in the reduction recognizer (which we do in our local project). But I'd really appreciate any suggestions for what I can do here.
Hi @aartbik, my apologies this got dragged out. I was out for a while.
After internal discussion we decided to also add the code to recognize min/max reductions. With that we can add all test cases to cover the initial additions in this current commit.
Since it has been a while, should I do an entirely new commit, or just upload a second patch here?
This update adds code to recognize min/max reductions. This was necessary to test part some of the functionality we had added initially.
Also, this update contains all the tests missing in the original commit.
Now that I know some testing part through recognition will follow, I am okay breaking up this revision as originally planned, i.e. adding the ops in a first small revision and then adding the recognition later.
That way you get the first revision in a bit quicker, since I think the recognition part may need some discussion.
|105 ↗||(On Diff #369596)|
please align parameters (clang-format will fix all those issues for you)
|129 ↗||(On Diff #369596)|
L108 to here could use some comments, in particular some visual representation of what you are looking for
|154 ↗||(On Diff #369596)|
IEEE floating-point comparisons as very tricky and we need to make sure we preserve the semantics when rewriting them into min/max . Have you made sure these rewriting preserve behaviors for e.g. NaN operands?