Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector type constructors #164

Open
wants to merge 9 commits into
base: development
Choose a base branch
from
Open

Conversation

Hugobros3
Copy link
Contributor

@Hugobros3 Hugobros3 commented Jul 3, 2024

The way vector types work in Thorin current matches LLVM: pointers and primitive types can be widened by setting a length field to a value other than 1. I believe this design is problematic and instead I suggest using vector type constructors (ie: vector_type(prim_type(i32), 4)).

Lots of code in Thorin is written with a questionable attention to flattened vector types, and patterns-match PrimType(i32, ...) as "a signed 32-bit integer" when it could in fact be a vector. By making the vector types use another constructor, there are no more risks of making this mistake. Making our type system syntax-directed prevents such oversights and better matches how the other backends deal with vectors (C, Shady, SPIR-V).

The new VectorType takes a ScalarType as a base. Only PrimType and PtrType inherit from ScalarType, matching the classic Thorin/LLVM behavior. For convenience, I added deconstruct_vector and a few more helper methods to be able to treat ScalarType and VectorType more or less as the old VectorType.

@Hugobros3 Hugobros3 requested a review from m-kurtenacker July 4, 2024 12:39
@Hugobros3 Hugobros3 force-pushed the vector-type-constructor branch from 07bd710 to 4f0756a Compare July 5, 2024 09:36
@Hugobros3 Hugobros3 force-pushed the vector-type-constructor branch from eee0899 to 06d83c2 Compare July 5, 2024 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants