Adds mappings between scalable MVT types and IR Types,
along with string printing. Fixes an issue reported by Jeroen.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
lib/CodeGen/ValueTypes.cpp | ||
---|---|---|
118 ↗ | (On Diff #149950) | Won't you need to have printing support for the generic scalable vector ? That should come here. |
Fixed string representation of scalable vectors which don't map to the existing defined set of MVTs.
lib/CodeGen/ValueTypes.cpp | ||
---|---|---|
193 ↗ | (On Diff #150303) | I can't find where these are defined, but given that the IR type is <scalable v4f32> should these names reflect that? "nx" is nice and short but may be confusing in that the mapping from "scalable" to "nx" isn't necessarily obvious. |
lib/CodeGen/ValueTypes.cpp | ||
---|---|---|
193 ↗ | (On Diff #150303) | As MC layer support for SVE started landing quite a while ago, the backend types too landed back then with names matching the then-current proposal for the IR types (<n x 4 x ...>). I agree it would be nice to have the backend type names match, but that's a bunch of churn, so it shouldn't be coupled to this patch and should wait until the IR type system changes are committed in case their name changes again. |
lib/CodeGen/ValueTypes.cpp | ||
---|---|---|
193 ↗ | (On Diff #150303) | Thanks, that makes sense to me. |
- Updated to head
- Removed MVT-specific ElementCount in favour of using the common struct
- Added unit tests.
LGTM, thanks!
llvm/lib/CodeGen/ValueTypes.cpp | ||
---|---|---|
208 | This isn't a comment on this patch per se but rather a question about this code overall. Why do we have all these vector and integer cases, for SVE and non-SVE? It seems like the default case should handle them all. I'm not at all saying you should change this. It's just an observation for a possible future NFC change from <someone>. |
This isn't a comment on this patch per se but rather a question about this code overall. Why do we have all these vector and integer cases, for SVE and non-SVE? It seems like the default case should handle them all.
I'm not at all saying you should change this. It's just an observation for a possible future NFC change from <someone>.