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

Support vector types in math operations #45

Closed
cpdt opened this issue Jun 21, 2018 · 3 comments
Closed

Support vector types in math operations #45

cpdt opened this issue Jun 21, 2018 · 3 comments

Comments

@cpdt
Copy link
Contributor

cpdt commented Jun 21, 2018

Is your feature request related to a problem? Please describe.
Yep -- in LLVM IR it is entirely valid to use mathematical operations on vector values, provided the two vectors have the same lengths and element type (and the element type is numeric, of course). However, all of Inkwell's math builder functions take and return IntValue or FloatValue values, meaning vectors can't easily be passed in without converting to a BasicValueEnum then back or using transmute.

Describe the solution you'd like
I'm not too sure what a solution would look like - a simple but messy solution might be to add functions like build_float_vec_add and build_int_vec_add, but you do lose the benefit of the typed parameters (although there's probably no way around that until #8 is resolved...)

I'm happy to make a pull request with the solution outlined above, although it's probably worth some discussion on a better alternative first?

@TheDan64
Copy link
Owner

Thanks for opening an issue!

If I understand correctly, for example, build_int_add (and friends) should not only take an IntValue<IntSize> but also a Vector<IntValue<IntSize>> (ideally). Can you add int and int vec? And likewise build_float_add (and friends) should not only accept FloatValue<FloatSize> but Vector<FloatValue<FloatSize>>? Again, can you add float to a float vector?

Assuming I'm understanding your feature request, I propose to do the following:

  1. v0.1.0: Create a new trait that is only implemented for IntValue and VectorValue (and likewise a trait for FloatValue and VectorValue) and use that trait in these math methods which will allow usage of vectors in addition to(pun intended) ints/floats as requested
  2. v0.2.0: Create a followup ticket for Sub-Types (Type Safety v1.1) #8 to further constrain the above traits to take subtypes into account: trait A is impl for IntValue and VectorValue<IntValue> (and likewise for floats)
  3. v0.2.0 or v0.3.0: (This might require const generics to be stable in rust, so inkwell version pending on that) Further constrain to IntValue<IntSize> and VectorValue<IntValue<IntSize>> (and likewise for floats)

How does that sound to you? I think that 1) will allow your feature request to be implemented in its most basic form and 2) allow the feature to acquire further compile time guarantees as inkwell is able to provide them

@cpdt
Copy link
Contributor Author

cpdt commented Jun 22, 2018

Hey! Thanks for the response :)

I don't think you're able to add a number to a vector - the LLVM language reference manual says "both arguments must have identical types" - which possibly complicates the first proposal... Maybe we can use a generic parameter there? i.e something like fn build_float_add<T: FloatMathValue>(&self, lhs: &T, rhs: &T, name: &str) -> T. I'll make a PR for this later today when I get the chance.

Those other proposals sound good... It might also be worth considering in the future an extra const generic parameter on VectorValue for the vector size (number of elements), since math operations require both vectors to have the same number of elements.

@TheDan64 TheDan64 added this to the 0.1.0 milestone Jun 22, 2018
@TheDan64
Copy link
Owner

Yes, generically constraining the lhs and rhs is definitely the way to go to ensure they're the same type then.

Sounds good, I look forward to your PR! In the mean time I will create another issue to make sure to follow up on 2) & 3). And good point on the vector size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants