I'm writing a small numeric library whose kernels run on the CPU, on CUDA and on Vulkan through the self-hosted SPIR-V backend, and I was making them generic over the float type. `f64` needs its own `exp` and `log` on the GPU, so I wrote them as polynomial series, and the `f64` Vulkan module stopped validating. This fixes the three backend bugs underneath that, and a crash I ran into after merging master.
The series take floats apart with `@bitCast` on vectors. Between vectors of the same length this reaches the Vulkan/OpenGL fallback in `bitCast`, which stores the source into a function variable of the destination type:
```zig
export fn main() callconv(.{ .spirv_kernel = .{ .x = 1, .y = 1, .z = 1 } }) void {
var x: @Vector(2, f32) = .{ 1, 2 };
_ = &x;
const bits: @Vector(2, i32) = @bitCast(x);
_ = bits;
}
```
```
$ zig build-exe -fno-llvm -target spirv64-vulkan -mcpu vulkan_v1_2 bitcast.zig -femit-bin=bitcast.spv
$ spirv-val --target-env vulkan1.2 bitcast.spv
error: line 45: OpStore Pointer <id> '36[%36]'s type does not match Object <id> '29[%29]'s type.
```
An elementwise vector bitcast is now a scalar `bitCast` per lane, the way `buildConvert` and friends already unroll vectors. Going through the scalar path also normalizes lanes of strange integers: a single `OpBitcast` on the whole vector validates for `@Vector(n, u3)` to `@Vector(n, i3)`, but leaves lanes that don't compare equal to the negative values they should hold.
The series also convert between integer and float lanes. A vector of one lane, or of more than four without `vector16`, is an array in SPIR-V, and `airFloatFromInt` and `airIntFromFloat` emitted a single `OpConvert*` with that array as its result type:
```zig
export fn main() callconv(.{ .spirv_kernel = .{ .x = 1, .y = 1, .z = 1 } }) void {
var x: @Vector(1, i32) = .{1};
_ = &x;
const y: @Vector(1, f32) = @floatFromInt(x);
_ = y;
}
```
```
$ spirv-val --target-env vulkan1.2 convert.spv
error: line 40: Expected float scalar or vector type as Result Type: ConvertSToF
%y = OpConvertSToF %_arr_f32_u32_1 %26
```
`@floatCast` and `@intCast` already go through `buildConvert`, which unrolls the vector; these two now do as well, with `buildConvert` picking the int/float conversion opcodes.
The coefficients sit in a comptime array that the series take as a comptime slice, and that alone fails to compile, although nothing reads the slice at runtime:
```zig
const weights = [_]f32{ 1, 2, 3 };
inline fn first(comptime w: []const f32) f32 {
return w[0];
}
export fn main() callconv(.{ .spirv_kernel = .{ .x = 1, .y = 1, .z = 1 } }) void {
var x: f32 = first(&weights);
_ = &x;
}
```
```
$ zig build-exe -fno-llvm -target spirv64-vulkan -mcpu vulkan_v1_2 dbgvar.zig -femit-bin=dbgvar.spv
dbgvar.zig:7:8: error: TODO (SPIR-V): pointer to constant 'dbgvar.weights'
```
It appears `airDbgVar` lowers the operand only to attach an `OpName` to it, and a pointer to a non-extern constant is still a TODO. A comptime-known value has no runtime variable of its own to name, so `airDbgVar` now leaves interned operands alone. The same thing held back 58 of the behavior tests skipped on SPIR-V, and the test commit un-skips them.
After merging master, four of those tests crashed the compiler in `structFieldPtr`, which has `unreachable` for packed containers since ebb8683ef1 ([#36996](https://codeberg.org/ziglang/zig/pulls/36996)):
```zig
const Flags = packed struct { a: bool, b: bool };
export fn main() callconv(.{ .spirv_kernel = .{ .x = 1, .y = 1, .z = 1 } }) void {
var flags: Flags = .{ .a = true, .b = false };
const b = &flags.b;
b.* = true;
}
```
```
thread 3961624 panic: reached unreachable code
src/codegen/spirv/CodeGen.zig:6906:27: 0x4767157 in structFieldPtr (main.zig)
.@"packed" => unreachable,
```
My reading is that the `unreachable` holds for the pointer derivations it was written for, since `constantPtr` now turns packed pointers into pointers to their host integer first, but `airStructFieldPtr` and `airStructFieldPtrIndex` still pass packed structs through. A pointer to a packed field is a bit-pointer into the struct's own host integer, so it is the struct pointer: that is what the old code computed, and what the C backend now does with `.begin`. Packed unions get back the TODO error they had before. I may be missing a plan for packed field pointers here, and I'm happy to change it if so. The commit also un-skips three packed tests that crash on master the same way. I suppose nothing caught this because those tests are skipped on SPIR-V and the `spirv64-vulkan` behavior target is commented out in `test/tests.zig`.
I ran the behavior tests through `tools/unskip_spirv_tests.zig` and [zig-spirv-test-executor](https://github.com/Snektron/zig-spirv-test-executor) on a Radeon 680M, and the new and un-skipped tests on an RTX 4060 as well. Every fix has tests that fail on master and pass with it; the vector ones compare lane by lane, since `expectEqual` on vectors doesn't compile for SPIR-V yet. Running each behavior file whole, nothing that passes on master fails. Five more skips already pass on master without these changes, so I left them out, but I can add them. The new vector tests copy the aarch64, arm, sparc64 and riscv64 skips of the tests around them, which I couldn't check.
Reviewed-on: https://codeberg.org/ziglang/zig/pulls/37040
Reviewed-by: Ali Cheraghi <alichraghi@noreply.codeberg.org>
And add an assertion to `Sema.pointerDerefExtra` to hopefully catch this
kind of bug a little more easily next time: this bug actually snuck past
our existing assertions in many cases!
Resolves: https://codeberg.org/ziglang/zig/issues/36452
These have no clear use case, no coherent definition, and incomplete/broken
compiler and linker support.
If actually needed, they can be reintroduced if someone can present a reasonable
use case along with a plan for how they should be implemented consistently in
all backends and linkers.
If I had to guess, `Sema.zirRoundOpType` was probably written when `@trunc` etc. could only have a float as their destination type. As a result, for the ints they can cast to now, they'd expect the expression inside them to be a `comptime_float`, which made `@trunc(@floor(runtime_float))` or `@trunc(@floatFromInt(runtime_int))` impossible.
By changing the returned type in this case to be generic poison, showing we don't know the expected type as *any* float can be converted to an int, `@trunc(@floor(runtime_float))` lowers to effectively `@floor(runtime_float)` and `@trunc(@floatFromInt(runtime_int))` throws the expected error that `@floatFromInt`'s result type is unknown here.
Fixes: https://codeberg.org/ziglang/zig/issues/32111
Co-authored-by: rue04 <rue04@rue04.de>
Reviewed-on: https://codeberg.org/ziglang/zig/pulls/35664
In legalization for packed struct init with an OPV field, we first try
to see if any field accounts for the entire bit size of the struct and
otherwise fall back to a sequence of bitcasts and shifts on each field
(added in fc1c83a363). However, OPV fields are not accounted for
in the fallback case, which means that codegen eventually panics when
seeing the bit size of 0.
This change makes it so that we ignore all OPV fields in
`packedAggregateInitBlockPayload` since they have no runtime bits.
A comptime-known `[]T` should behave as similarly as possible to a comptime-
known `*[n]T`. It is now possible to coerce the former to the latter if
their length matches.
A comptime-known `[]T` should behave as similarly as possible to a comptime-
known `*[n]T`. It is now possible to directly dereference both to an array.
Also adds checks for slices with undefined length in code paths where a
slice is dereferenced as an array which would previously crash.
* `tools/generate_c_size_and_align_checks.zig`: superceded by any cbe
output artifact
* `cTypePreferredAlignment`: no uses, if brought back, cbe should be
updated to verify `__alignof`
Closes#19991Closes#20881Closes#35523
and remove "release" from the enum tag names.
no functional change, however, despite the addition of
backwards-compatibile declarations in this patch, it is breaking because
expressions that use `==` or `!=` operators will not able to use the
deprecated names.
I predict these names to survive until the 1.0 tag without any more
breakage.
`IntType.fromType` would previously set the signedness of any backing int
of a `packed union` to `unsigned` unconditionally which caused comparisons
of packed unions with large, signed backing integers to miscompile.
Packed unions are now lowered correctly to their actual backing int type.
This commit implements the `@backingInt` and `@fromBackingInt` builtins.
These builtins are meant to replace the now-deprecated `@intFromEnum` and
`@enumFromInt` builtins.
`@backingInt` works with all enums and with bitpacks with *explicit* backing
integer types only. It also works with tagged unions, returning the backing
integer of the active tag value.
An `undefined` enum or bitpack yields an `undefined` backing integer.
The builtin is lowered to `bit_cast`.
`@fromBackingInt` infers its result type, which may be any enum or a bitpack
with an *explicit* backing integer type. It takes a parameter of exactly
that backing integer type.
For enums, passing a backing integer that is either `undefined` or would
yield an invalid tag value results in safety-checked Illegal Behavior.
For bitpacks, passing an `undefined` backing integer yields an `undefined`
bitpack.
The builtin is lowered to `bit_cast` or `bit_cast_safe` (more details below).
`@bitCast` now also performs a safety check for invalid tag values if its
destination type is an `enum`.
This commit also introduces a new AIR instruction, `bit_cast_safe`.
Similarly to `int_cast_safe`, it includes a safety check for invalid enum
tag values. When `@enumFromInt` is eventually removed, `int_cast_safe` won't
need to perform this safety check anymore.
Legalize only implements `expand_bit_cast_safe`, and no additional insts
to scalarize `bit_cast_safe` with vector/array operands. That is because
it can just reuse the existing scalarizations for regular `bit_cast`.
The safety check mandated by `bit_cast_safe` is only desired if the result
type is a scalar enum tag value, so the scalarization process is exactly
equivalent. The only difference is that we have to perform a safety check
on the result of the scalarization.
This means that there's no reason for a backend to want scalarization of
regular `bit_cast`, but not of `bit_cast_safe` (or the other way round).
If a backend is having trouble with the `bit_cast_safe` safety check it
should just use `expand_bit_cast_safe` instead.
Also adds a `std.meta.BackingInt()` function to conveniently get the result
type of `@backingInt` and for use in the langref.
Empty enums are uninstantiable, so their backing integer should be too.
This avoids a bunch of weird special cases during semantic analysis.
If you were previously using `enum {}`, your code should continue to work
fine.
If you were previously using `enum(uN) {}` (where `uN` is any integer type),
you'll probably want to use `enum(uN) { _ }` instead.
Jacob Young