Skip to content

remove the type parameter from builtins that support vectors #6835

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.

Sign up for GitHub

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

Jump to bottom
Closed
data-man opened this issue Oct 27, 2020 · 6 comments · Fixed by #12574
Closed

remove the type parameter from builtins that support vectors #6835

data-man opened this issue Oct 27, 2020 · 6 comments · Fixed by #12574
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone
0.10.0

Comments

@data-man
Copy link
Contributor

data-man commented Oct 27, 2020 •
edited
Loading

As described in LLVM's docs:

This is an overloaded intrinsic. You can use llvm.* on any integer bit width, or on any vector with integer elements.

@bitReverse(comptime T: type, integer: T) T uses llvm-bitreverse-intrinsics
@clz(comptime T: type, integer: T) uses llvm-ctlz-intrinsic
@ctz(comptime T: type, integer: T) uses llvm-cttz-intrinsic
@popCount(comptime T: type, integer: T) uses llvm-ctpop-intrinsic

Proposal: remove first parameter (like @cos, @sin, etc. builtins):

@bitReverse(value: anytype) @TypeOf(value)
@clz(value: anytype) T
@ctz(value: anytype) T
@popCount(value: anytype) T

Where T is comptime_int if value is known at comptime. Otherwise, T is usize.

Related #903 as always.
@jedisct1
Copy link
Contributor

Yup, that would be neat.

@LemonBoy
Copy link
Contributor

The proposal here is to revert #2240, check out #2119 for the rationale.

@data-man
Copy link
Contributor Author

The proposal here is to revert #2240

Yes, partially.
The main wish is to support vectors.

@LemonBoy
Copy link
Contributor

The main wish is to support vectors.

As they're defined right now it'd be a problem to extend them to vectors.
The documentation states about @ctz:

If integer is known at comptime, the return type is comptime_int. Otherwise, the return type is an unsigned integer with the minimum number of bits that can represent the bit count of the integer type.

If this logic is extended to vectors @ctx(v4i32) gives you a v4i5 that's essentially useless (LLVM will happily produce scalar code for you but you lose every kind of parallelism) until you intCast it back to a wider type that the CPU supports.

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Oct 28, 2020
@Vexu Vexu added this to the 0.8.0 milestone Oct 28, 2020
@data-man
Copy link
Contributor Author

data-man commented Nov 4, 2020 •
edited
Loading

The same proposal for @addWithOverflow, @subWithOverflow, @mulWithOverflow and @shlWithOverflow.

@LemonBoy LemonBoy mentioned this issue Feb 2, 2021
Closed
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk added the accepted This proposal is planned. label Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk mentioned this issue Nov 30, 2021
Closed
@andrewrk andrewrk changed the title Support integer vectors by some builtins remove the type parameter from builtins that support vectors Jan 13, 2022
andrewrk added a commit that referenced this issue Jan 13, 2022
@andrewrk
stage2: implement @ctz and @clz including SIMD
93b854e 
AIR:
 * `array_elem_val` is now allowed to be used with a vector as the array
   type.
 * New instructions: splat, vector_init

AstGen:
 * The splat ZIR instruction uses coerced_ty for the ResultLoc, avoiding
   an unnecessary `as` instruction, since the coercion will be performed
   in Sema.
 * Builtins that accept vectors now ignore the type parameter. Comment
   from this commit reproduced here:

   The accepted proposal #6835 tells us to remove the type parameter from
   these builtins. To stay source-compatible with stage1, we still observe
   the parameter here, but we do not encode it into the ZIR. To implement
   this proposal in stage2, only AstGen code will need to be changed.

Sema:
 * `clz` and `ctz` ZIR instructions are now handled by the same function
   which accept AIR tag and comptime eval function pointer to
   differentiate.
 * `@typeInfo` for vectors is implemented.
 * `@splat` is implemented. It takes advantage of `Value.Tag.repeated` 😎
 * `elemValue` is implemented for vectors, when the index is a scalar.
   Handling a vector index is still TODO.
 * Element-wise coercion is implemented for vectors. It could probably
   be optimized a bit, but it is at least complete & correct.
 * `Type.intInfo` supports vectors, returning int info for the element.
 * `Value.ctz` initial implementation. Needs work.
 * `Value.eql` is implemented for arrays and vectors.

LLVM backend:
 * Implement vector support when lowering `array_elem_val`.
 * Implement vector support when lowering `ctz` and `clz`.
 * Implement `splat` and `vector_init`.
@andrewrk
Copy link
Member

andrewrk commented Jan 14, 2022 •
edited
Loading

The same proposal for @addWithOverflow, @subWithOverflow, @mulWithOverflow and @shlWithOverflow.

These are covered by #10248.

scorphus pushed a commit to scorphus/zig that referenced this issue Jan 15, 2022
@andrewrk
stage2: implement @ctz and @clz including SIMD
b6f7b90 
AIR:
 * `array_elem_val` is now allowed to be used with a vector as the array
   type.
 * New instructions: splat, vector_init

AstGen:
 * The splat ZIR instruction uses coerced_ty for the ResultLoc, avoiding
   an unnecessary `as` instruction, since the coercion will be performed
   in Sema.
 * Builtins that accept vectors now ignore the type parameter. Comment
   from this commit reproduced here:

   The accepted proposal ziglang#6835 tells us to remove the type parameter from
   these builtins. To stay source-compatible with stage1, we still observe
   the parameter here, but we do not encode it into the ZIR. To implement
   this proposal in stage2, only AstGen code will need to be changed.

Sema:
 * `clz` and `ctz` ZIR instructions are now handled by the same function
   which accept AIR tag and comptime eval function pointer to
   differentiate.
 * `@typeInfo` for vectors is implemented.
 * `@splat` is implemented. It takes advantage of `Value.Tag.repeated` 😎
 * `elemValue` is implemented for vectors, when the index is a scalar.
   Handling a vector index is still TODO.
 * Element-wise coercion is implemented for vectors. It could probably
   be optimized a bit, but it is at least complete & correct.
 * `Type.intInfo` supports vectors, returning int info for the element.
 * `Value.ctz` initial implementation. Needs work.
 * `Value.eql` is implemented for arrays and vectors.

LLVM backend:
 * Implement vector support when lowering `array_elem_val`.
 * Implement vector support when lowering `ctz` and `clz`.
 * Implement `splat` and `vector_init`.
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
Vexu added a commit to Vexu/zig that referenced this issue Aug 21, 2022
@Vexu
stage2+stage1: remove type parameter from bit builtins
544f1a6 
Closes ziglang#12529
Closes ziglang#12511
Closes ziglang#6835
Vexu added a commit to Vexu/zig that referenced this issue Aug 21, 2022
@Vexu
stage2+stage1: remove type parameter from bit builtins
019a541 
Closes ziglang#12529
Closes ziglang#12511
Closes ziglang#6835
@Vexu Vexu mentioned this issue Aug 22, 2022
Merged
@andrewrk andrewrk modified the milestones: 0.11.0, 0.10.0 Aug 24, 2022
TUSF pushed a commit to TUSF/zig that referenced this issue May 9, 2024
@Vexu
stage2+stage1: remove type parameter from bit builtins
290b63f 
Closes ziglang#12529
Closes ziglang#12511
Closes ziglang#6835
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Milestone
0.10.0
Development

Successfully merging a pull request may close this issue.

stage2+stage1: remove type parameter from bit builtins Vexu/zig
5 participants
@LemonBoy @andrewrk @jedisct1 @data-man @Vexu