zig
zig copied to clipboard
Plan9: Fix The Backend
This adds
- unnamed decl support
- relocation support Hello world now works again
CI checks failing
Yes, I saw that. I am not sure why they are failing because I didn't change anything in codegen relating to branches and cannot repro this on a personal computer. I'll look into it soon, thanks for bringing it to my attention.
Overflow error is due to an error in consolidating instructions between parent, then, and else branches in airCondBr
lowering. This usually can be one of two things: either a bug in consolidation code, or an incorrect/incomplete implementation of some lowering for the target. You should first rule out this issue can be reproed on x86_64-linux
- if yes, it's a wider bug in the codegen. Otherwise, could it be that you have missed some Plan9 specific prong(s) in the codegen?
Funnily enough, I get this on master also when doing zig build test-compare
: https://paste.rs/m0G
And this is without the Plan9 test case in there. I couldn't repro this with build-exe.
@g-w1 related question, do you want to type up some release notes for zig's plan9 support to go into the upcoming 0.10.0 release notes?
Sure, but I don't know if I will have anything to say in the release notes. If the CI bug on this PR gets resolved, then I will have something to type up. Thanks for asking!
Blocking on #13231 Which I believe is the same bug as the one on the CI here.
Rebasing on latest master makes the original panic go away which is quite worrying and I'll investigate locally why that is. In the meantime, in order not to block your PR, there are a couple more fixes required to make the CI green. First though, could you please rebase on latest master? Next, when you build the compiler with LLVM enabled, it assumes Plan9 is a valid target of LLVM's and tries to build libc.a
and libcompiler_rt.a
. As we currently lack any way of doing it for Plan9 we need to set Plan9 as incapable of building any of those libs until further notice.
I tried pushing the fixes to you branch directly but you have not allowed it in your fork sadly, so I am pasting the diff here:
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -1088,10 +1088,10 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
// Once they are capable this condition could be removed. When removing this condition,
// also test the use case of `build-obj -fcompiler-rt` with the native backends
// and make sure the compiler-rt symbols are emitted.
- const capable_of_building_compiler_rt = build_options.have_llvm;
+ const capable_of_building_compiler_rt = build_options.have_llvm and options.target.os.tag != .plan9;
- const capable_of_building_zig_libc = build_options.have_llvm;
- const capable_of_building_ssp = build_options.have_llvm;
+ const capable_of_building_zig_libc = build_options.have_llvm and options.target.os.tag != .plan9;
+ const capable_of_building_ssp = build_options.have_llvm and options.target.os.tag != .plan9;
const comp: *Compilation = comp: {
// For allocations that have the same lifetime as Compilation. This arena is used only during this
diff --git a/src/link.zig b/src/link.zig
index b0c829448..dc1174992 100644
--- a/src/link.zig
+++ b/src/link.zig
@@ -581,7 +581,7 @@ pub const File = struct {
.macho => return @fieldParentPtr(MachO, "base", base).updateDeclLineNumber(module, decl),
.c => return @fieldParentPtr(C, "base", base).updateDeclLineNumber(module, decl),
.wasm => return @fieldParentPtr(Wasm, "base", base).updateDeclLineNumber(module, decl),
- .plan9 => @panic("TODO: implement updateDeclLineNumber for plan9"),
+ .plan9 => return @fieldParentPtr(Plan9, "base", base).updateDeclLineNumber(module, decl),
.spirv, .nvptx => {},
}
}
diff --git a/src/link/Plan9.zig b/src/link/Plan9.zig
index 86c3c1817..7e7e7be90 100644
--- a/src/link/Plan9.zig
+++ b/src/link/Plan9.zig
@@ -956,6 +956,14 @@ pub fn allocateDeclIndexes(self: *Plan9, decl_index: Module.Decl.Index) !void {
_ = self;
_ = decl_index;
}
+
+/// Must be called only after a successful call to `updateDecl`.
+pub fn updateDeclLineNumber(self: *Plan9, mod: *Module, decl: *const Module.Decl) !void {
+ _ = self;
+ _ = mod;
+ _ = decl;
+}
+
pub fn getDeclVAddr(
self: *Plan9,
decl_index: Module.Decl.Index,
diff --git a/test/cases/plan9/exit.zig b/test/cases/plan9/exit.zig
index 735818fb8..5c5cf2176 100644
--- a/test/cases/plan9/exit.zig
+++ b/test/cases/plan9/exit.zig
@@ -1,5 +1,5 @@
pub fn main() void {}
// run
-// target=x86_64-plan9,aarch64-plan9
+// target=x86_64-plan9
//
Please note I've had to remove aarch64-plan9
as you haven't updated that backend for lowering unnamed consts for Plan9 target.
Re the original error (overflow): it was caused because we tried building libcompiler_rt.a
with x86_64
backend which not advanced enough and an overflow in airCondBr
is usually caused by an unhandled lowering. Applying the above patch I suggested fixes it for me too regardless of rebasing.
Thank you so much @kubkon . I'll try to apply those changes as soon as possible. I'll also take a look if rebasing fixes #13231 and close it if necessary
Thank you so much @kubkon . I'll try to apply those changes as soon as possible. I'll also take a look if rebasing fixes #13231 and close it if necessary
I don't think your PR and #13231 are actually related beyond hitting the same panic message which can easily be triggered by trying out more behavior tests with x86 backend :-)
Oh, I see. Thanks anyways. I think I have checked the box to allow maintainers to make edits, so I am not sure what is going on.