stage1: Expand SysV C ABI support for small structs

[bndbsh]

While the SysV ABI is not that complicated, LLVM does not allow us
direct access to enforce it. By mimicking the IR generated by clang,
we can trick LLVM into doing the right thing. This involves two main
additions:

1. AGG ABI class
   This is not part of the spec, but since we have to track class per
   eightbyte and not per struct, the current enum is not enough. I
   considered adding multiple classes like: INTEGER_INTEGER,
   INTEGER_SSE, SSE_INTEGER. However, all of those cases would trigger
   the same code path so it's simpler to collapse into one. This class is
   only used on SysV.

2. LLVM C ABI type
   Clang uses different types in C ABI function signatures than the
   original structs passed in, and does conversion. For example, this
   struct: { i8, i8, float } would use { i16, float } at ABI boundaries.
   When passed as an argument, it is instead split into two arguments i16
   and float. Therefore, for every struct that passes ABI boundaries we
   need to keep track of its corresponding ABI type. Here are some more
   examples:

| Struct             | ABI equivalent |
| { i8, i8, i8, i8 } | i32            |
| { float, float }   | double         |
| { float, i32, i8 } | { float, i64 } |

Then, we must update function calls, returns, parameter lists and inits
to properly convert back and forth as needed.

[bndbsh]

I did more testing and cleaned up the code a little bit, I haven't run into any more issues so far. Since at least the int support is needed for clang.zig, things tended to break spectacularly when there was an issue. I've gotten quickjs to work with this patch, which was the original motivator.

This should tick off one box in #1481.

[bndbsh]

Looks like the change I made to make sure it works on windows broke windows, classic. Gonna try to setup an environment to debug.

[Jarred-Sumner]

I've gotten quickjs to work with this patch, which was the original motivator.

This PR would also help with embedding JavaScriptCore's C++ implementation (not the glib/objc api). JavaScriptCore uses several stack-allocated objects that occupy <= 16 bytes

[andrewrk]

Really nice work, thank you for this @bndbsh.

[ShoshinX]

Hi,

Thanks for the work :). I tried testing it out using the following code

const test_t = extern struct {
    // words: [1]u8 doesn't work too
    words: [1]c_ulong,
};

//This is similar to using export
fn accept_return_struct(w: test_t) callconv(.C) test_t {
    return w;
}

test {
    const a = test_t{
        .words = .{1},
    };
    _ = accept_return_struct(a);
}

It doesn't work with structs that has an array whose size is <= 16 bytes, but expanding the array so that it's > 16 bytes works. So I don't think #1481 's x86_64: struct & union return values <= 16 bytes is complete yet.

[bndbsh]

I didn't think to test arrays in the struct at all. Did you happen to compare what clang does in this case? With something like clang -S -emit-llvm test.c.

[ShoshinX]

I made a test.c file that's as close to the zig example I posted:

struct test {
    unsigned long words[1];
};

typedef struct test test_t;

test_t accept_return_struct(test_t w){
    return w;
}

int main() {
    test_t a = {.words = {1}};
    accept_return_struct(a);
}

And here is the test.ll which clang -S -emit-llvm test.c outputted, which I don't really understand yet

; ModuleID = 'test.c'
source_filename = "test.c"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-linux-gnu"

%struct.test = type { [1 x i64] }

@__const.main.a = private unnamed_addr constant %struct.test { [1 x i64] [i64 1] }, align 8

; Function Attrs: noinline nounwind optnone sspstrong uwtable
define dso_local i64 @accept_return_struct(i64 %0) #0 {
  %2 = alloca %struct.test, align 8
  %3 = alloca %struct.test, align 8
  %4 = getelementptr inbounds %struct.test, %struct.test* %3, i32 0, i32 0
  %5 = bitcast [1 x i64]* %4 to i64*
  store i64 %0, i64* %5, align 8
  %6 = bitcast %struct.test* %2 to i8*
  %7 = bitcast %struct.test* %3 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %6, i8* align 8 %7, i64 8, i1 false)
  %8 = getelementptr inbounds %struct.test, %struct.test* %2, i32 0, i32 0
  %9 = bitcast [1 x i64]* %8 to i64*
  %10 = load i64, i64* %9, align 8
  ret i64 %10
}

; Function Attrs: argmemonly nofree nosync nounwind willreturn
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #1

; Function Attrs: noinline nounwind optnone sspstrong uwtable
define dso_local i32 @main() #0 {
  %1 = alloca %struct.test, align 8
  %2 = alloca %struct.test, align 8
  %3 = bitcast %struct.test* %1 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %3, i8* align 8 bitcast (%struct.test* @__const.main.a to i8*), i64 8, i1 false)
  %4 = getelementptr inbounds %struct.test, %struct.test* %1, i32 0, i32 0
  %5 = bitcast [1 x i64]* %4 to i64*
  %6 = load i64, i64* %5, align 8
  %7 = call i64 @accept_return_struct(i64 %6)
  %8 = getelementptr inbounds %struct.test, %struct.test* %2, i32 0, i32 0
  %9 = bitcast [1 x i64]* %8 to i64*
  store i64 %7, i64* %9, align 8
  ret i32 0
}

attributes #0 = { noinline nounwind optnone sspstrong uwtable "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #1 = { argmemonly nofree nosync nounwind willreturn }

!llvm.module.flags = !{!0, !1, !2}
!llvm.ident = !{!3}

!0 = !{i32 1, !"wchar_size", i32 4}
!1 = !{i32 7, !"PIC Level", i32 2}
!2 = !{i32 7, !"PIE Level", i32 2}
!3 = !{!"clang version 12.0.1"}

[omgitsmoe]

Could this be related?

-        HPDF_Page_GetCurrentTextPos((_HPDF_Dict_Rec *)page)    {x=50.6879997 y=841.000000 }    _HPDF_Point
        x    50.6879997    float
        y    841.000000    float
const point = HPDF_Page_GetCurrentTextPos(page);
-        point    {x=841.000000 y=0.00000000 }    .cimport:1:20.struct__HPDF_Point
        x    841.000000    float
        y    0.00000000    float

typedef float   HPDF_REAL;
typedef struct _HPDF_Point {
    HPDF_REAL  x;
    HPDF_REAL  y;
} HPDF_Point

Using @bitOffsetOf in Zig returns:
X Offset 0, Y Offset 32

Also tested it with commit ed174b7, which includes this PR.

Emulating the function in Zig works just fine

fn getCurrentTextPos(page: HPDF_Page) !Point {
    if (HPDF_Page_Validate(page) == 0 or page == null) {
        return PDFGenerator.Error.PDFError;
    }

    // page is a C pointer, so we have to deref it and to be able to access a field
    // (imitates page->attr)
    const attr = @ptrCast(HPDF_PageAttr, @alignCast(@alignOf(HPDF_PageAttr), page.*.attr));
    return Point{ .x = attr.*.text_pos.x, .y = attr.*.text_pos.y };
}

Point X 50.6879997253418 Y 841

[Jarred-Sumner]

possibly related, but not sure since the struct is > 16 bytes: #9487

[bndbsh]

@ShoshinX thanks for the reduction. The issue is here:

zig/src/stage1/analyze.cpp

Line 8763 in e5e6ced

return_elem_types[i] = llvm_sse_for_size(type_sizes[i]);

The code only considers ints and floats, and the arrays fall into the else as floats. That function would have to be updated to take arrays into account and compute the type accordingly.

@omgitsmoe that looks like it should work, however you're on windows and I didn't add a test case for a struct containing only floats so I never checked it on windows. I suspect that using double at the abi boundary (which is what the PR does for _HPDF_Point) instead of 2 x float might be at fault here, but I can't verify until I get to a windows setup. The same struct works on linux.

Either way, looks like resolve_llvm_c_abi_type has to be little bit smarter with regards to the contents of each eightbyte. I might take a stab at it next week or so.