This is an archive of the discontinued LLVM Phabricator instance.

[SingleSource/Vectorizer] Add unit tests for index-select pattern.
ClosedPublic

Authored by fhahn on Feb 6 2023, 3:13 AM.

Details

Summary

Dedicated unit tests for loops which select the index of the minimum
value.

This adds runtime test coverage for D132063.

Depends on D143377.

Diff Detail

Repository
rT test-suite

Event Timeline

fhahn created this revision.Feb 6 2023, 3:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 6 2023, 3:13 AM
fhahn requested review of this revision.Feb 6 2023, 3:13 AM
This revision is now accepted and ready to land.Feb 6 2023, 5:59 AM

Overall good, but I recommend adding a test case for MinIdx = -1.

Meinersbur accepted this revision.Feb 21 2023, 10:00 PM

LGTM, but some description of what is tested would be useful. I would not expect people to search for the initial commit, find D132063 in the comment, and hope that Phabricator is still up.

SingleSource/UnitTests/Vectorizer/index-select.cpp
32

Could you add a comment on what the point of running on sorted data over random data is? Isn't it pointwise anyway?

100

Some description of what is being tested would be useful. min_index_select_u32_u32_start_0 isn't very descriptive.

fhahn marked 2 inline comments as done.Apr 23 2023, 2:52 AM

Overall good, but I recommend adding a test case for MinIdx = -1.

Thanks, I added a test for that and a few other variants (different induction starts and Min values).

LGTM, but some description of what is tested would be useful. I would not expect people to search for the initial commit, find D132063 in the comment, and hope that Phabricator is still up.

Thanks, I added a description to the commit message.

SingleSource/UnitTests/Vectorizer/index-select.cpp
32

I added some comments here to describe what's being tested. I also changed it to use std::numeric_limits<Ty>::max() instead of -1 with unsigned types.

100

Thanks, I added a description in the committed version.

This revision was automatically updated to reflect the committed changes.
fhahn marked 2 inline comments as done.