A handful of changes to get the regressed behavior tests running again.
- Replace `decorateBlockOffsets` with a recursive function `decorateLayout`
that walks arrays, vectors, structs, unions, optionals, and error unions,
emitting `ArrayStride` and member `Offset` decorations at every
level. Previously we weren't handling nested types.
- Restrict `Block` decoration to struct types with
`uniform`, `push_constant`, `storage_buffer` storage classes.
Previously we decorated through every pointer, contaminating the cached struct
type so the same shape used as a stack local also picked up `Block`.
- Lower `ptr_slice_ptr_ptr` and `ptr_slice_len_ptr`
- No longer emit a redundant `**T` typed `OpVariable` for function parameters.
Logical addressing also forbids such variables.
- Eliminate dead code from invocation globals unreachable from any entry point.
Reverting the workaround in `lib/std/start.zig`.
Execution modes (e.g. `LocalSize`, `OriginUpperLeft`) were previously set
via `gpu.executionMode()`, which used inline assembly to emit `OpExecutionMode`.
The SPIR-V assembler now rejects this instruction and retrieves execution mode information
from function cc, deleting `gpu.executionMode()` entirely.
Two new `spirv_task` and `spirv_mesh` calling conventions are also added
and `PackedCallingConvention.unpack()` now takes a trailing data slice.
Closes#35240Fixes#35238Fixes#35259
Supported types are:
- `OpTypeSampler`
- `OpTypeImage`
- `OpTypeSampledImage`
- `OpTypeRuntimeArray` with indexing and `.len` field
The SPIR-V backend is bit-rotted so behavior tests no longer pass (compiler crashes).
However I've verified the new added tests are passing.
All public codegen and linker APIs now use the following error set:
Allocator.Error || Io.Cancelable || error{AlreadyReported}
This is defined as `link.Error` and aliased as `codegen.Error`.
The compiler "backend" (including both codegen and linker) has a fairly
limited set of failure modes. Most of them are as follows:
* Bad inline assembly (codegen)
* Output file I/O error (link)
* Symbol with no definition or multiple definitions (link)
* Relocation error, e.g. overflow (link)
* Unimplemented/unsupported feature (codegen/link)
* `error.OutOfMemory` (codegen/link)
* `error.Canceled` (codegen/link)
The last two cases are special, because they are possible across most of
the compiler codebase and have fixed code paths for handling---e.g.
`error.OutOfMemory` almost always calls `Compilation.setAllocFailure`,
and `error.Canceled` should typically propagate all the way up the call
stack.
However, all of the other cases should follow the same general pattern:
the codegen/linker implementation should mark an error on `Compilation`
somehow (either through `link_diags` or `failed_codegen`), and return
`error.AlreadyReported`. This error code indicates that an operation
failed, but that the caller does not need to take action to recover,
because the failure has already been recorded in a way which will be
expressed to the user. For instance, the codegen error set used to
contain `error.Overflow`, but this case should have already been
reported to the user, because implementation-agnostic code cannot know
how best to express the failure to the user.
The `error.AlreadyReported` error code encompasses what was previously
represented by multiple errors, including `error.CodegenFail`,
`error.LinkFailure`, and `error.AnalysisFail`. It is not necessary to
differentiate these cases. (Not to be confused with the usage of
`error.AnalysisFail` in the compiler *frontend*, i.e. `Sema`, which is
unchanged by this diff.)
Because the backend error set is now very small, it is easy to
exhaustively `switch` on, and also many functions can be given concrete
error sets. There are no longer different error sets for different
operations. (As an exception, I have not tackled `link.File.open` in
this diff, which has a big inferred error set where I think most errors
are reported to the user with an `else => |e|` case.)
I have also changed how `link.MappedFile`, our abstraction for handling
the awkward "moving things around" aspect of incremental linking, does
error handling. The public API of this abstraction now uses the
following small error set:
Allocator.Error || Io.Cancelable || error{MappedFileIo}
The first two cases are self-explanatory. Then, `error.MappedFileIo` is
a catch-all error code encompassing that an error was encountered when
accessing the underlying `Io.File`. This error may have occurred when
attempting to flush the file to disk, or to change its size, etc. In
this case, the *specific* error---which was actually returned from the
`Io.File` API---is available in the `MappedFile.io_err.?` field. Linker
implementations which use `MappedFile` (currently `Elf2` and `Coff`)
should eventually handle `error.MappedFileIo` by reporting an error in
`Compilation.link_diags`, including that specific I/O error in the error
message.
The rationale here is essentially that error codes returned from
functions exist for control flow purposes, and a linker implementation
should not care about the distinction between file I/O error codes (e.g.
`error.DiskQuota` vs `error.InputOutput`). The only reason it needs the
concrete error is to expose it to the user. Therefore, by wrapping these
failure modes under `error.MappedFileIo`, we keep the error set actually
used by the linker implementation as small as possible, which encourages
avoiding patterns like `else => |e| diags.fail(...)` (which is
potentially dangerous since it may prevent `error.OutOfMemory` or
`error.Canceled` from propagating correctly). It additionally helps
linker implementations maintain more precise error messages---for
instance, if `Elf2` encounters `error.NoSpaceLeft` writing to the output
file, the error will now read "failed to write output file: NoSpaceLeft"
instead of something more generic like "prelink failed: NoSpaceLeft".
Finally, while working on these refactors, I was able to eliminate those
pesky `src_loc: Zcu.LazySrcLoc` parameters from the codegen and link
logic. These parameters did not make sense, because at this point in the
compiler pipeline, we do not have precise source location
information---so these parameters were always passed as just the
location of the function declaration, or as some placeholder
(`.unneeded` or the top of `lib/std/std.zig`) if there wasn't an
appropriate function definition. The only logic which actually *uses*
these source locations is codegen implementations' `fail` functions. Per
my earlier list of failure modes, those functions are called in two main
cases:
* Bad inline assembly
* Unimplemented/unsupported feature
The first case should be moved to the compiler frontend (tracked by
https://github.com/ziglang/zig/issues/10761), while the second case is
essentially a compiler TODO so it is permissible for it to have
imprecise error reporting. Therefore, codegen implementations' `fail`
functions now just call `navSrcLoc` themselves when necessary, which
(once we make improvements to inline assembly) will only happen when
there is a deficiency in the codegen implementation.
I was careful to make `Elf2` and `Coff` error reporting work well in
this diff, by using no `else` case other than `else => |e| return e`
when `switch`ing on errors and by always handling `error.MappedFileIo`
by unwrapping `MappedFile.io_err.?`. I also added concrete error sets to
almost every function in `Elf2`. (That was, um, actually why I started
this diff, because it was annoying me that I wouldn't get as many
compile errors at once...)
This PR merges the functionality of the `getLastOrNull` method into `getLast`, which improves consistency as its
based on methods like `front`, `back`, and `peek` in the `Deque` and `PriorityQueue` containers.
Reviewed-on: https://codeberg.org/ziglang/zig/pulls/32008
Reviewed-by: Andrew Kelley <andrew@ziglang.org>
Also, notably, remove `Air.value`! The `onePossibleValue` check was
actually dead code, because it is a bug if Sema ever emits code which
considers a value of OPV type to be runtime-known---and at that point
`Air.value` is just a thin wrapper around `Air.Ref.toInterned`.
I've realised that the cause of at least some of our weird CI flakiness
was a bug in how `Nav` values were resolved. Consider this scenario: the
frontend resolves the type of a `Nav`, and then sends a function to the
backend, which requires the backend to lower a pointer to that `Nav`.
The backend calls `InternPool.getNav` to determine the `Nav`'s type.
However, this races with the frontend resolving the *value* of that
`Nav`. This involves writing separately to two fields, `bits` and
`type_or_value`. If only one of these changes is observed, then the
backend will incorrectly interpret the type as the value or vice versa,
leading to a crash or even a miscompilation. (Of course, there's also
the straightforward issue that the racing loads were non-atomic, making
them illegal).
The only good solution to this was to make `Nav` 4 bytes bigger, giving
it separate `type` and `value` fields. In theory that's a quite small
change, but it ended up having a bunch of nice consequences which led to
this diff being a bit bulkier than expected:
* `Nav.Repr.Bits` was simplified, because it no longer has to track
"resolution status": we can use `.none` for that. This frees up some
bits to make things more consistent between the "type resolved" and
"fully resolved" states.
* This consistency allowed the `Nav.status` union to be replaced with a
simpler field `Nav.resolved`, which is a bit nicer to work with.
* Most of the "getter" functions were able to be removed from `Nav`
because the state they were fetching had been moved to simple fields
on `Nav.resolved`.
* There were still a handful of free bits in `Nav.Repr.Bits`, which
could be used to represent the "const" and "threadlocal" flags rather
than these being stored on `Key.Extern` and `Key.Variable`. This is a
bit more convenient for linkers.
* With those bits gone, `Key.Variable` is a trivial wrapper around a
type and an initial value, and the fact that a declaration is mutable
can be represented solely through the "const" flag. Therefore,
`Key.Variable` no longer served a purpose, and could be eliminated
entirely in favour of storing the variable's initial value directly in
the "value" field of the `Nav`.
So, I'm quite pleased with this refactor! But anyway, regarding the bug
fix which actually motivated this: if I've done my job correctly, this
should solve some crashes, such as these (which were what tipped me off
to this bug in the first place):
https://codeberg.org/ziglang/zig/actions/runs/2306/jobs/7/attempt/1https://codeberg.org/ziglang/zig/actions/runs/2173/jobs/6/attempt/1
...and, who knows, perhaps even the random SIGSEGVs we've seen on some
targets! Probably not, but one can hope.
The change in codegen/x86_64/CodeGen.zig was not strictly necessary (the
Sema change I did solves the error I was getting there), I just think
it's better style anyway.
Now that https://github.com/ziglang/zig/issues/24657 has been
implemented, the compiler can simplify its internal representation of
comptime-known `packed struct` and `packed union` values. Instead of
storing them field-wise, we can simply store their backing integer
value. This simplifies many operations and improves efficiency in some
cases.
Eliminate the `std.Thread.Pool` used in the compiler for concurrency and
asynchrony, in favour of the new `std.Io.async` and `std.Io.concurrent`
primitives.
This removes the last usage of `std.Thread.Pool` in the Zig repository.
I started this diff trying to remove a little dead code from the C
backend, but ended up finding a bunch of dead code sprinkled all over
the place:
* `packed` handling in the C backend which was made dead by `Legalize`
* Representation of pointers to runtime-known vector indices
* Handling for the `vector_store_elem` AIR instruction (now removed)
* Old tuple handling from when they used the InternPool repr of structs
* Straightforward unused functions
* TODOs in the LLVM backend for features which Zig just does not support
they seem to be always `null` even when accessed through extern key so we have no way to tell whether they have natural alignment or not to decorate. And the reason we don't always decorate them is because some environments might be too dumb and crash for this.
Ali Cheraghi