intrinsic-test: add lots of documentation#2124
Conversation
|
r? @adamgemmell rustbot has assigned @adamgemmell. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
Improve documentation for Arm intrinsic filtering - why we test some intrinsics and not others.
This flag was missing documentation but the other flags had documentation
It isn't necessarily obvious what this function does
davidtwco
force-pushed
the
intrinsic-test-docs
branch
from
4b84ed2 to
770f7ce
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| 1 => VALUES_8[index % 2].into(), | ||
| 2 => VALUES_8[index % 4].into(), | ||
| 3 => VALUES_8[index % 8].into(), | ||
| 4 => VALUES_8[index % 16].into(), | ||
| 5 => VALUES_5[index % VALUES_5.len()].into(), | ||
| 6 => VALUES_6[index % VALUES_6.len()].into(), | ||
| 7 => VALUES_7[index % VALUES_7.len()].into(), |
There was a problem hiding this comment.
These lines are very weird, I bet they're dead code 🤔
|
LGTM - David & I have the same employer, but I'm going to merge this as it's just doc comments without functionality |
I'm fine with this when the changes aren't likely to be controversial and you haven't been involved in writing the changes. |
3 participants
When I had tried to make quick changes to
intrinsic-testin the past, I found it quite an unintuitive codebase - lots of the code generation is split over multiple functions that make it hard to see the big picture of what is generated (particularly without running the tool and looking at what is generated), and lots of functions are confusingly named. Prior to doing any refactorings in pursuit of adding SVE testing, I wanted to first document as much of the codebase as I could to make it easier to understand - I'll follow-up with renames and refactorings.