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

stage1: Implement @reduce builtin for vector types #6558

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

LemonBoy
Copy link
Contributor

@LemonBoy LemonBoy commented Oct 4, 2020

The builtin folds a Vector(N,T) into a scalar T using a specified operator.

Closes #2698

The builtin folds a Vector(N,T) into a scalar T using a specified
operator.

Closes ziglang#2698
@alexnask alexnask added the stage1 The process of building from source via WebAssembly and the C backend. label Oct 4, 2020
@andrewrk
Copy link
Member

andrewrk commented Oct 5, 2020

My goodness, we just cannot catch a break with that windows CI

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice progress! 1 nitpick and then this is good to land.

BTW why not work towards this in stage2? This is still useful progress because the changes to builtin.zig, behavior tests, and zig_llvm.{h,cpp} will persist, but I have a feeling you would enjoy the work more on stage2, and once we get far enough on that, the current plan is to delete the stage1 codebase (#6378).

Also I'm happy to do the nitpick myself in the merge process if you want, just say the word.

Comment on lines +104 to +108
And,
Or,
Xor,
Min,
Max,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower case tag names for new enums please. I normally don't want to care about style stuff but I also don't want to have to make unnecessary breaking changes later.

@LemonBoy
Copy link
Contributor Author

LemonBoy commented Oct 5, 2020

BTW why not work towards this in stage2?

Because I was playing with SIMD rather than with the compiler :^)
When the LLVM backend for stage2 is kickstarted I'll definitely move to the Zig side of the moon.

Also I'm happy to do the nitpick myself in the merge process if you want, just say the word.

Yeah go ahead, didn't know about the lowercase rule for enum members.

@andrewrk
Copy link
Member

andrewrk commented Oct 5, 2020

Oh, hmm. Gonna change my mind on this one. Capitalized enum tag names in this case works around the conflicts with the language keywords. 🤷

@andrewrk andrewrk merged commit 22b5e47 into ziglang:master Oct 5, 2020
@andrewrk
Copy link
Member

andrewrk commented Oct 5, 2020

When the LLVM backend for stage2 is kickstarted I'll definitely move to the Zig side of the moon.

Alright well that's certainly motivating for me or someone else to get started on #6541 :-)

@data-man
Copy link
Contributor

@LemonBoy

llvm/IR/IRBuilder.h also has CreateAddReduce and CreateMulReduce
Can you implement .Add and .Mul, please?

@LemonBoy
Copy link
Contributor Author

Can you implement .Add and .Mul, please?

Sure, I didn't implement those because I wasn't sure about the names to use (nor I had any need for those ops).
Since the add/sub/mul provided by LLVM are wrapping I'd go for Wrapping{Add,Mul}.

@data-man
Copy link
Contributor

data-man commented Nov 1, 2020

@LemonBoy

Sure

Before 0.7.0 release? :)

But I thought about integer/float overflow for .Add and .Mul. :(
E.g. if a vector is float32 type, can @reduce return float64?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@reduce builtin
4 participants