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>
* `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
The SPIR-V backend previously ran codegen single-threaded in the linker thread.
Now each codegen job creates an `Mir` like other self-hosted backends.
Other changes:
- Bring back `dedup_types.zig` and `prune_unused.zig` **ISel**s which were originaly
removed because codegen was single-threaded at that time and therefor had no use
- Clean up `BinaryModule.zig`
- Remove `checkLogicalPtrOperation` from `elemPtrOneLayerOnly` in `Sema.zig`.
Element access uses `OpAccessChain`, which works on all logical address spaces
without `VariablePointers`. the check is only needed for pointer arithmetic.
The bugs here actually exist on master branch too, but they are being
caught by the new static assertions which check type size and alignment.
It turns out that MSVC's struct/union "pack" pragma and its "align"
declspec interact in undocumented ways which are extremely problematic
for generated code. Solving this will require changing how the C backend
lowers various types; the disabled tests are all tagged unions, but
there are also issues with structs with underaligned fields which the
behavior tests just happen to not currently be triggering.
Also remove `@frameSize`, closing #3654.
While the other machinery might remain depending on #23446, it is
settled that there will not be `async`/ `await` keywords in the
language.
This was done by regex substitution with `sed`. I then manually went
over the entire diff and fixed any incorrect changes.
This diff also changes a lot of `callconv(.C)` to `callconv(.c)`, since
my regex happened to also trigger here. I opted to leave these changes
in, since they *are* a correct migration, even if they're not the one I
was trying to do!
The compiler actually doesn't need any functional changes for this: Sema
does reification based on the tag indices of `std.builtin.Type` already!
So, no zig1.wasm update is necessary.
This change is necessary to disallow name clashes between fields and
decls on a type, which is a prerequisite of #9938.
What is `sparcel`, you might ask? Good question!
If you take a peek in the SPARC v8 manual, §2.2, it is quite explicit that SPARC
v8 is a big-endian architecture. No little-endian or mixed-endian support to be
found here.
On the other hand, the SPARC v9 manual, in §3.2.1.2, states that it has support
for mixed-endian operation, with big-endian mode being the default.
Ok, so `sparcel` must just be referring to SPARC v9 running in little-endian
mode, surely?
Nope:
* https://github.com/llvm/llvm-project/blob/40b4fd7a3e81d32b29364a1b15337bcf817659c0/llvm/lib/Target/Sparc/SparcTargetMachine.cpp#L226
* https://github.com/llvm/llvm-project/blob/40b4fd7a3e81d32b29364a1b15337bcf817659c0/llvm/lib/Target/Sparc/SparcTargetMachine.cpp#L104
So, `sparcel` in LLVM is referring to some sort of fantastical little-endian
SPARC v8 architecture. I've scoured the internet and I can find absolutely no
evidence that such a thing exists or has ever existed. In fact, I can find no
evidence that a little-endian implementation of SPARC v9 ever existed, either.
Or any SPARC version, actually!
The support was added here: https://reviews.llvm.org/D8741
Notably, there is no mention whatsoever of what CPU this might be referring to,
and no justification given for the "but some are little" comment added in the
patch.
My best guess is that this might have been some private exercise in creating a
little-endian version of SPARC that never saw the light of day. Given that SPARC
v8 explicitly doesn't support little-endian operation (let alone little-endian
instruction encoding!), and no CPU is known to be implemented as such, I think
it's very reasonable for us to just remove this support.
This is a misfeature that we inherited from LLVM:
* https://reviews.llvm.org/D61259
* https://reviews.llvm.org/D61939
(`aarch64_32` and `arm64_32` are equivalent.)
I truly have no idea why this triple passed review in LLVM. It is, to date, the
*only* tag in the architecture component that is not, in fact, an architecture.
In reality, it is just an ILP32 ABI for AArch64 (*not* AArch32).
The triples that use `aarch64_32` look like `aarch64_32-apple-watchos`. Yes,
that triple is exactly what you think; it has no ABI component. They really,
seriously did this.
Since only Apple could come up with silliness like this, it should come as no
surprise that no one else uses `aarch64_32`. Later on, a GNU ILP32 ABI for
AArch64 was developed, and support was added to LLVM:
* https://reviews.llvm.org/D94143
* https://reviews.llvm.org/D104931
Here, sanity seems to have prevailed, and a triple using this ABI looks like
`aarch64-linux-gnu_ilp32` as you would expect.
As can be seen from the diffs in this commit, there was plenty of confusion
throughout the Zig codebase about what exactly `aarch64_32` was. So let's just
remove it. In its place, we'll use `aarch64-watchos-ilp32`,
`aarch64-linux-gnuilp32`, and so on. We'll then translate these appropriately
when talking to LLVM. Hence, this commit adds the `ilp32` ABI tag (we already
have `gnuilp32`).
Reorganize how the binOp and genBinOp functions work.
I've spent quite a while here reading exactly through the spec and so many
tests are enabled because of several critical issues the old design had.
There are some regressions that will take a long time to figure out individually
so I will ignore them for now, and pray they get fixed by themselves. When
we're closer to 100% passing is when I will start diving into them one-by-one.
Brian Ferri