Fix vararg ABI

[kinke]

Requires ldc-developers/ldc#768 and addresses ldc-developers/ldc#702 and ldc-developers/ldc#73 (for Win64 only!).

[kinke]

This reduces the failing tests to 41 on Win64:

core.thread (SEGFAULT)
std.algorithm (Failed)
std.complex (Failed)
std.conv (Failed)
std.csv (Failed)
std.math (Failed)
std.numeric (Failed)
std.parallelism (SEGFAULT)
std.path (SEGFAULT)
std.process (Failed)
std.regex (SEGFAULT)
std.stdio (Failed)
std.stream (SEGFAULT)
std.string (SEGFAULT)
std.uni (SEGFAULT)
std.uri (SEGFAULT)
std.zlib (SEGFAULT)
std.net.isemail (SEGFAULT)
std.internal.math.errorfunction (Failed)
std.internal.math.gammafunction (Failed)
std.algorithm-debug (Failed)
std.complex-debug (Failed)
std.conv-debug (Failed)
std.csv-debug (Failed)
std.datetime-debug (SEGFAULT)
std.json-debug (Failed)
std.math-debug (Failed)
std.numeric-debug (Failed)
std.parallelism-debug (SEGFAULT)
std.path-debug (SEGFAULT)
std.process-debug (Failed)
std.regex-debug (SEGFAULT)
std.socket-debug (Failed)
std.stdio-debug (Failed)
std.stream-debug (SEGFAULT)
std.uni-debug (SEGFAULT)
std.uri-debug (SEGFAULT)
std.zlib-debug (SEGFAULT)
std.net.isemail-debug (SEGFAULT)
std.internal.math.errorfunction-debug (Failed)
std.internal.math.gammafunction-debug (Failed)

[kinke]

extern(D) doesn't seem to be too way off. This:

import core.vararg;
import core.stdc.stdio;

extern(C) void logC(const(char)* str, ...)
{
    va_list args;
    va_start(args, str);
    vprintf(str, args);
    va_end(args);
}

void logD(const(char)* str, ...)
{
    va_list args;
    va_start(args, str);
    assert(args == _argptr);

    foreach (ti; _arguments)
        printf("  %s [size %d]\n", ti.toString().ptr, ti.tsize);
    vprintf(str, _argptr);
}

void main()
{
    immutable fl = 3.0f;
    logC  ("logC:   %d %f %f %lld %f %s\n", 1, 2.0, fl, 0x4_0000_0000L, fl + 2.0f, "lala".ptr);
    logD  ("logD:   %d %f %f %lld %f %s\n", 1, 2.0, fl, 0x4_0000_0000L, fl + 2.0f, "lala".ptr);
    printf("printf: %d %f %f %lld %f %s\n", 1, 2.0, fl, 0x4_0000_0000L, fl + 2.0f, "lala".ptr);
}

yields:

logC:   1 2.000000 3.000000 17179869184 5.000000 lala
  int [size 4]
  double [size 8]
  immutable(float) [size 4]
  long [size 8]
  float [size 4]
  immutable(char)* [size 8]
logD:   1 2.000000 0.000000 17179869184 0.000000 lala
printf: 1 2.000000 3.000000 17179869184 5.000000 lala

So there's a problem with floats for extern(D). It looks as if floats are never passed directly for varargs and converted to double instead. E.g., there's no printf type specifier distinguishing between float and double (only a l prefix for longdouble). This explains the produced .ll output:

define void @_D5hello4logDFPxaYv({ i64, %object.TypeInfo** } %._arguments, i8* noalias nocapture %._argptr, i8* %str_arg) #0 {
    ...
}

define i32 @_Dmain({ i64, { i64, i8* }* } %unnamed) #0 {
  %fl = alloca float, align 4
  %_argptr_storage = alloca { i64, double, i64, i64, i64, i8* }
  store float 3.000000e+00, float* %fl
  %1 = load float* %fl
  call void (i8*, ...)* @logC(i8* getelementptr inbounds ([29 x i8]* @.str2, i32 0, i32 0), i32 1, double 2.000000e+00, double 3.000000e+00, i64 17179869184, double 5.000000e+00, i8* getelementptr inbounds ([5 x i8]* @.str3, i32 0, i32 0))
  %2 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 0
  %3 = bitcast i64* %2 to i32*
  store i32 1, i32* %3
  %4 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 1
  store double 2.000000e+00, double* %4
  %5 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 2
  %6 = bitcast i64* %5 to float*
  store float 3.000000e+00, float* %6
  %7 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 3
  store i64 17179869184, i64* %7
  %8 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 4
  %9 = bitcast i64* %8 to float*
  store float 5.000000e+00, float* %9
  %10 = getelementptr { i64, double, i64, i64, i64, i8* }* %_argptr_storage, i32 0, i32 5
  store i8* getelementptr inbounds ([5 x i8]* @.str4, i32 0, i32 0), i8** %10
  %11 = load { i64, %object.TypeInfo** }* @._arguments.array
  %12 = bitcast { i64, double, i64, i64, i64, i8* }* %_argptr_storage to i8*
  call void @_D5hello4logDFPxaYv({ i64, %object.TypeInfo** } %11, i8* noalias nocapture %12, i8* getelementptr inbounds ([28 x i8]* @.str5, i32 0, i32 0))
  %tmp = call i32 (i8*, ...)* @printf(i8* getelementptr inbounds ([29 x i8]* @.str6, i32 0, i32 0), i32 1, double 2.000000e+00, double 3.000000e+00, i64 17179869184, double 5.000000e+00, i8* getelementptr inbounds ([5 x i8]* @.str7, i32 0, i32 0))
  ret i32 0
}

i.e., the 2 floats are passed to logC() as double literals while they are stored in a 64-bit slot for the _argptr_storage struct without converting to double before. If we want C-ABI compliance in this case, we'd have to convert all floats to double, involving the TypeInfo[] _arguments parameter as well.

The next thing to be tested are structs/static arrays > 64 bits passed to a vararg extern(D) function. The core.stdc.stdarg.va_arg() functions assume the struct is passed byval (pointer to hidden copy) as that's what's done for the C ABI. I fear putting one into the _argptr_storage struct for extern(D) currently means that either the struct is allocated directly in there or that a pointer to it (the original struct, not a hidden copy => byref passing, ldc-developers/ldc#172) is stored.

[kinke]

structs are currently stored directly inside the _argptr_storage struct, so the va_arg() functions fail horribly for bigger ones. This can be tested by inspecting the .ll output of:

import core.vararg;
import core.stdc.stdio;

struct Small { long a; }
struct Big { long a, b; }

void variadic(T)(...)
{
    foreach (ti; _arguments)
        printf("  %s [size %d]\n", ti.toString().ptr, ti.tsize);
    static if (!is(T == struct))
    {
        auto format = "%s " ~ (__traits(isFloating, T) ? "%f" : "%d") ~ "\n";
        printf(format.ptr, typeid(T).toString().ptr, va_arg!T(_argptr));
    }
}

void main()
{    
    variadic!int(1);
    variadic!double(2.0);
    variadic!float(3.0f);

    Small small = { 4L };
    variadic!Small(small);

    Big big = { 5L, 6L };
    variadic!Big(big);
}

At least for x86_64, it seems it would be much easier and cleaner if we declared a variadic D function as a normal LLVM (C) variadic function, except for the injected TypeInfo[] parameter, and so got rid of the _argptr_storage struct. The _argptr pointer would be allocated and initialized to what the LLVM va_start intrinsic returns at the beginning of the variadic D function body. Argument transformations (byval struct passing, cast to int...) would need to be moved to the call sites (at least for the optional ones), fixing ldc-developers/ldc#172 at the same time.

I don't know how compatible other platforms would be to such an approach, but iirc, @redstar mentioned that the PowerPC vararg ABI is similar.

[dnadlinger]

At least for x86_64, it seems it would be much easier and cleaner if we declared a variadic D function as a normal LLVM (C) variadic function

Yes, that's been my plan/suggestion all along. Sorry if I was ambiguous.

[kinke]

For all platforms? Or are there any legacy considerations, e.g., for 32-bit x86?

[kinke]

Edited title and description to reflect the LDC changes in ldc-developers/ldc#768.

[kinke]

Added a generic experimental version of both files, currently lacking x86_64 support for non-Windows.

[dnadlinger]

I'd rather actually make this __va_list* or something so that code that assumes x86-style variadics just doesn't compile instead of crashing horribly at runtime.

[kinke]

Hmm I see the point, but there's a problem with the hidden argptr. It is currently defined as Type::tvalist, i.e., char, see gen/target.cpp:81. If we change it to a hardcoded core.vararg.__va_list_ for System V, I guess the user must import core.vararg when implementing a variadic extern(D) function, even when not accessing _argptr at all.

[dnadlinger]

Okay, I see the problem. We should probably fix this at some point by moving __va_list into object and properly initializing Type::tvalist, but that can wait for now.