Skip to content
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.

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

stage1 C ABI compatibility #1481

Closed
8 of 11 tasks
andrewrk opened this issue Sep 6, 2018 · 79 comments
Closed
8 of 11 tasks

stage1 C ABI compatibility #1481

andrewrk opened this issue Sep 6, 2018 · 79 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Sep 6, 2018

This issue is to track C ABI compatibility support in the stage1 compiler.

Here's what support looks like currently:

  • integers, floats, pointers, enums, bools work on every target, as parameters and return values
  • x86_64: struct & union parameters & return values larger than 16 bytes
  • x86_64: struct & union parameters that have at least 1 integer in them
  • x86_64: struct & union parameters <= 16 bytes which break structs into parameters
  • x86_64: struct & union parameters <= 16 bytes which are all floats
  • x86_64: struct & union return values <= 16 bytes
  • ARM: struct & union return values
  • ARM: struct & union parameters
  • C ABI for parameters that is unsupported gives a compile error linking to this issue
  • C ABI for return values that is unsupported gives a compile error linking to this issue
  • other architectures structs & unions parameters & return values

For those who find this issue from the compile error, leave a comment detailing your specific needs and I'll see if I can code those up for you to unblock you, so you don't have to wait for this issue to be 100% solved.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior stage1 The process of building from source via WebAssembly and the C backend. labels Sep 6, 2018
@andrewrk andrewrk added this to the 0.3.0 milestone Sep 6, 2018
andrewrk added a commit that referenced this issue Sep 7, 2018
@andrewrk
Copy link
Member Author

andrewrk commented Sep 7, 2018

Update:

I just merged a branch into master which does the following things:

  • Unsupported C ABI gives a compile error, like this:
const ASmallUnion = extern union {
    r: u8,
};
extern fn foo(x: ASmallUnion) void;

pub fn main() void {
    const x = ASmallUnion{ .r = 0x12 };

    foo(col);
}
/home/andy/downloads/zig/build/test.zig:4:8: error: TODO: support C ABI for more targets. https://github.com/ziglang/zig/issues/1481
extern fn foo(x: ASmallUnion) void;
       ^
/home/andy/downloads/zig/build/test.zig:4:8: note: pointers, integers, floats, bools, and enums work on all targets
extern fn foo(x: ASmallUnion) void;
       ^
  • There is a new type of test, C ABI tests, which covers the C ABI that we do suport. See test/stage1/c_abi/* for the source.
Test 1/9 C importing Zig ABI Tests...OK
Test 2/9 C ABI integers...OK
Test 3/9 C ABI floats...OK
Test 4/9 C ABI pointer...OK
Test 5/9 C ABI bool...OK
Test 6/9 C ABI array...OK
Test 7/9 C ABI big struct...OK
Test 8/9 C ABI big union...OK
Test 9/9 C ABI small struct of ints...OK
  • Added all targets C ABI support for integers, floats, bools, enums, and pointers
  • Added x86_64 C ABI support for big structs and unions (which are passed by memory)
  • Added x86_64 C ABI support for small structs which contain at least one integer

andrewrk added a commit that referenced this issue Sep 8, 2018
also panic instead of emitting bad code for returning small structs

See #1481
@andrewrk
Copy link
Member Author

andrewrk commented Sep 8, 2018

Zig will now either panic or emit a compile error when C ABI compatibility is required that it doesn't support yet. Therefore this issue is now an enhancement in order to provide additional C ABI compatibility and not a bug.

For those who find this issue from the compile error, leave a comment detailing your specific needs and I'll see if I can code those up for you to unblock you, so you don't have to wait for this issue to be 100% solved.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed bug Observed behavior contradicts documented or intended behavior labels Sep 8, 2018
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0, 1.1.0 Sep 8, 2018
@bheads
Copy link

bheads commented Oct 5, 2018

Running into this issue when trying to work with the godot native headers.
"TODO implement C ABI for x86_64 return types. type 'godot_string'"

https://github.com/GodotNativeTools/godot_headers

andrewrk added a commit that referenced this issue Oct 13, 2018
* add __multi3 compiler rt function. See #1290
* compiler rt includes ARM functions for thumb and aarch64 and
  other sub-arches left out. See #1526
* support C ABI for returning structs on ARM. see #1481
@DavidYKay
Copy link

Running into this when bringing up the BGFX rendering library on 64-bit Linux.

My code calls the following function:

bgfx_create_vertex_buffer()

Error message:

TODO implement C ABI for x86_64 return types. type 'struct_bgfx_vertex_buffer_handle_s'

bgfx_vertex_buffer_handle_s is defined in this header, using a macro.

The macro evaluates to the following struct definition:

typedef struct bgfx_vertex_buffer_handle_s { 
    uint16_t idx; 
} bgfx_vertex_buffer_handle_t

This seems to correspond to "x86_64: struct & union return values <= 16 bytes," above.

Please advise on how to get unblocked. 😃

Thanks much for your hard work!

@zacharycarter
Copy link

zacharycarter commented Dec 15, 2018

I'm using BGFX as well - not sure if @DavidYKay 's issue will affect me as well, but it will most likely end up blocking me too.

Trying to use the CSFML Network API -

const std = @import("std");
const defines = @import("defines.zig");
const ZealErrors = @import("error.zig").ZealErrors;
const sfml = @cImport({
    // See https://github.com/zig-lang/zig/issues/515
    @cDefine("_NO_CRT_STDIO_INLINE", "1");
    @cInclude("SFML/Network.h");
});

var listener: ?*sfml.sfTcpListener = undefined;

pub fn init() u8 {
    listener = sfml.sfTcpListener_create();
    if (listener == null) {
        return defines.QUIT_FAILURE;
    }

    var socket_status = sfml.sfTcpListener_listen(listener, 8080, sfml.sfIpAddress_Any);
    
    return defines.QUIT_SUCCESS;
}

I'm using the CSFML library - csfml: stable 2.4 (bottled), HEAD

I installed via homebrew on macOS High Sierra 10.13.6 (17G65)

Here is the related header file(s) - https://github.com/SFML/CSFML/blob/master/include/SFML/Network/TcpListener.h

Here's the offending function signature written in Zig - pub extern fn sfTcpListener_listen(listener: ?*sfTcpListener, port: c_ushort, address: sfIpAddress) sfSocketStatus;

And the error -

/Users/zachcarter/projects/zeal_zig/src/console_server.zig:23:5: error: TODO: support C ABI for more targets. https://github.com/ziglang/zig/issues/1481
pub extern fn sfTcpListener_listen(listener: ?*sfTcpListener, port: c_ushort, address: sfIpAddress) sfSocketStatus;
    ^
/Users/zachcarter/projects/zeal_zig/src/console_server.zig:23:5: note: pointers, integers, floats, bools, and enums work on all targets
pub extern fn sfTcpListener_listen(listener: ?*sfTcpListener, port: c_ushort, address: sfIpAddress) sfSocketStatus;

And here's the above example, without using @cimport -

const std = @import("std");
const defines = @import("defines.zig");
const ZealErrors = @import("error.zig").ZealErrors;

pub const struct_sfTcpListener = @OpaqueType();
pub const sfTcpListener = struct_sfTcpListener;

pub const sfIpAddress = extern struct {
    address: [16]u8,
};

pub const sfSocketStatus = extern enum {
    sfSocketDone = 0,
    sfSocketNotReady = 1,
    sfSocketPartial = 2,
    sfSocketDisconnected = 3,
    sfSocketError = 4,
};

pub extern const sfIpAddress_Any: sfIpAddress;

pub extern fn sfTcpListener_create() ?*sfTcpListener;
pub extern fn sfTcpListener_listen(listener: ?*sfTcpListener, port: c_ushort, address: sfIpAddress) sfSocketStatus;

var listener: ?*sfTcpListener = undefined;

pub fn init() u8 {
    listener = sfTcpListener_create();
    if (listener == null) {
        return defines.QUIT_FAILURE;
    }

    var socket_status = sfTcpListener_listen(listener, 8080, sfIpAddress_Any);
    
    return defines.QUIT_SUCCESS;
}

@nebulaeonline
Copy link

nebulaeonline commented Dec 29, 2018

Trying to run @sizeof builtin on packed structs from 2 bytes to 8 bytes, compiler emits the following: error: TODO: support C ABI for more targets. #1481

This is the relevant code (from x86_64.zig):

// Segment Selector
pub const SegmentSelector = packed struct {
    rpl: u2,
    ti: u1,
    index: u13,
};

From kzig.zig:

const m1 = "sizeof(SegmentSelector) == %lu\n";
efi_result = klib.uefi.AsciiPrint(&m1, @sizeOf(x86.SegmentSelector));

@ThatsNoMoon
Copy link

ThatsNoMoon commented Jan 13, 2019

I'm trying to use XCB, translating this hello world program into Zig.

Here's what I have so far:

const std = @import("std");
const xcb = @cImport({
    @cInclude("xcb/xcb.h");
});

pub fn main() error!void {
    std.debug.warn("Starting...\n");
    const connection = xcb.xcb_connect(null, null);
    defer xcb.xcb_disconnect(connection);

    const screen = xcb
        .xcb_setup_roots_iterator(xcb.xcb_get_setup(connection)).data;

    const window = xcb.xcb_generate_id(connection);

    xcb.xcb_create_window(connection,
                          xcb.XCB_COPY_FROM_PARENT,
                          window,
                          screen.root,
                          0, 0,
                          150, 150,
                          10,
                          xcb.XCB_WINDOW_CLASS_INPUT_OUTPUT,
                          screen.root_visual,
                          0, null);

    xcb.xcb_map_window(connection, window);

    xcb.xcb_flush(connection);

    const stdin = try std.io.getStdIn();
    stdin.readByte();
}

And the error message:

TODO implement C ABI for x86_64 return types. type 'struct_xcb_screen_iterator_t'

Referring to the type of screen.

Also, I am a newcomer to Zig, so it's possible I'm doing something wrong.

Thank you for your time!

@xxxbxxx
Copy link
Contributor

xxxbxxx commented Sep 5, 2019

trying to use https://github.com/cimgui/cimgui
many functions take a struct parameter:

struct ImVec2
{
    float x, y;
};

@shawnl
Copy link
Contributor

shawnl commented Oct 3, 2019

Vector ABI depends on the compiler flags on x86_64, and current the c_abi tests have the compiler flags mixed up so that the zig version expects arguments to be passed in registers (such as %ymm0 and %zmm0 that don't exist on earlier CPUs, which only have %xmm0 et cetera) and the c version expects things to be passed on the stack.

But clearly the C can also pass on the stack with certain arguments https://godbolt.org/z/RCPb2c , we are passing -march=native so I don't know what we get the stack behavior https://godbolt.org/z/DzE_Px

@IgorKorobeynikov
Copy link

when will these goals be achieved?

@nektro
Copy link
Contributor

nektro commented Jun 26, 2022

when stage2/0.10.0 is released

@a99984b1799
Copy link

a99984b1799 commented Aug 18, 2022

Getting this using zig-imgui (project code here):

./src/positions.zig:89:39: error: TODO: support C ABI for more targets. https://github.com/ziglang/zig/issues/1481
            _ = ig.Text("%s", position.ticker);
                                      ^

Where:

pub const Holding = struct {
    name: []const u8,
    ticker: []const u8,
[...]

Builds on Windows but not on Linux, both running Zig 0.10.0-dev.3559+d2342370f.

Edit Working around this circumstance with position.ticker.ptr :)

@Vexu
Copy link
Member

Vexu commented Dec 7, 2022

No longer relevant, test coverage in zig build test-c-abi.

@Vexu Vexu closed this as completed Dec 7, 2022
@Vexu Vexu modified the milestones: 1.1.0, 0.11.0 Dec 7, 2022
@esnunes
Copy link

esnunes commented Dec 7, 2022

No longer relevant, test coverage in zig build test-c-abi.

Hey @Vexu , I didn't get why you've closed this issue, is the stage1 fully compatible now? I'm especially interested in the ARM: struct & union parameters issue.

@Vexu
Copy link
Member

Vexu commented Dec 7, 2022

Stage1 no longer exists.

@IgorKorobeynikov
Copy link

😳

@IgorKorobeynikov
Copy link

@andrewrk ARM: struct & union parameters fixed now? In stage2 compiller?

@Vexu
Copy link
Member

Vexu commented Dec 7, 2022

All but complex number tests are passing for arm, if you find some case that doesn't work report it as a new bug.

@IgorKorobeynikov
Copy link

Ok

@esnunes
Copy link

esnunes commented Dec 7, 2022

All but complex number tests are passing for arm, if you find some case that doesn't work report it as a new bug.

You are talking about the master version, right? Thanks

@Vexu
Copy link
Member

Vexu commented Dec 7, 2022

Most of the ABI improvements should have already landed in 0.10.0 (for self-hosted).

@rofrol
Copy link
Contributor

rofrol commented Aug 28, 2023

What about tasks?

  • x86_64: struct & union return values <= 16 bytes
  • ARM: struct & union parameters
  • other architectures structs & unions parameters & return values

They are unchecked in @andrewrk's post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. stage1 The process of building from source via WebAssembly and the C backend.
Projects
Status: Nice to have
Development

No branches or pull requests