Re: Fix D compilation on Solaris

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Hi Iain,

> On Wed, 31 Oct 2018 at 10:40, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>>
>> Hi Iain,
>>
>> > My first suspect here would be 'struct UnionExp', see d/dmd/expression.h
>> >
>> > Upstream dmd use a poor man's alignment, from what I recall to be
>> > compatible with the dmc compiler.
>> >
>> >         // Ensure that the union is suitably aligned.
>> >         real_t for_alignment_only;
>> >
>> > What happens if you were to replace that with marking the type as
>> > __attribute__ ((aligned (8))) ?
>>
>> thanks for the suggestion: this worked just fine.  After a couple more
>> libphobos adjustments (described below), I was able to finish the build
>> on both sparc-sun-solaris2.11 and i386-pc-solaris2.11.
>>
>> The link tests still all fail as before, but sparc and x86 are now on
>> par here :-)
>>
>> Here are the new issues I saw while completing the sparc build:
>>
>> * math.d has two problems on 32-bit sparc:
>>
>> /vol/gcc/src/hg/trunk/local/libphobos/src/std/math.d:5278:18: error:
>> undefined identifier 'ControlState'
>>  5278 |     ControlState savedState;
>>       |                  ^
>> /vol/gcc/src/hg/trunk/local/libphobos/src/std/math.d:5325:25: error:
>> undefined identifier 'ControlState'
>>  5325 |     static ControlState getControlState() @trusted nothrow @nogc
>>       |                         ^
>> /vol/gcc/src/hg/trunk/local/libphobos/src/std/math.d:5390:17: error:
>> undefined identifier 'ControlState'
>>  5390 | static void setControlState(ControlState newState) @trusted
>> nothrow @nogc
>>       |                 ^
>>
>>   Fixed by using the ControlState alias on both SPARC64 and SPARC.
>>
>> /vol/gcc/src/hg/trunk/local/libphobos/src/std/math.d:5211:9: error: static assert  "Not implemented for this architecture"
>>  5211 |         static assert(false, "Not implemented for this architecture");
>>       |         ^
>>
>>   Similarly, ExceptionMask was only defined for SPARC64.  However,
>>   looking closer it seems that the current definition only matches Linux
>>   resp. Glibc fenv.h (FE_*).  The Solaris values are different on sparc.
>>
>>   This seems to be a recurring theme, unfortunately: definitions guarded
>>   by version ($CPU) are really CRuntime_Glibc && $CPU.  I fear libphobos
>>   has to be way more careful to distinguish between definitions that
>>   only depend on the target cpu and those that are (also) OS-dependent.
>>
>
> The phobos part is largely free of version (CPU), the druntime
> bindings are in a little better shape, despite there being a lot of
> places dotted around the place that need to be checked.
>
>>   Instead of hardcoding all this, it may be worth having a look at how
>>   Go handles this: they dump the definitions with gcc
>>   -fdump-go-spec=tmp-gen-sysinfo.go and postprocess them in
>>   libgo/mksysinfo.sh.  This way, such errors and potential
>>   inconsistencies are avoided from the start.  This would also massivly
>>   simplify work for potential porters.
>>
>
> You mean a tool such as htod (header to D)?  I don't think that has
> ever gained traction in upstream.

that's a pity.  It certainly would simplify things...

> The existing C bindings for sure are brittle, and never take into
> account changes between versions (FreeBSD comes to mind), but for new
> ports, it at least tries to be consistent in that unhandled places
> always end with a 'static assert (false)' compiler error.  If I was
> just to defend the current status quo a little.

I see.  Types and interfaces changing between versions can certainly be
a problem.  However, Solaris is usually very careful not to make
incompatible changes.

>> * My previous patch had a typo, now also fixed:
>>
>> /vol/gcc/src/hg/trunk/local/libphobos/libdruntime/core/sys/posix/ucontext.d:984:20:
>> error: undefined identifier 'uint32_t'
>>   984 |       uint32_t[32] fpu_regs;
>>       |                    ^
>>
>> While those were enough to finish the build, I noticed a couple of
>> additional issues:
>>
>> * During make check, part (or all) of libphobos was rebuilt.  I strongly
>>   suspect that this happens because contrib/gcc_update doesn't handle
>>   libphobos yet: it needs to touch generated files to avoid exactly this
>>   sort of problem.  I'll post a separate patch once tested.
>>
>> * Unlike the gdc.dg tests, many gdc.test tests appear as UNRESOLVED like
>>   this:
>>
>> UNRESOLVED: runnable/A16.d   compilation failed to produce executable
>>
>>   There's no preceding FAIL for the link failure itself.  Besides, the
>>   testname needs to include the gdc.test prefix.
>>
>> * One issue I forgot last time: when defining the SPARC64 struct
>>   fpregset_t in libdruntime/core/sys/posix/ucontext.d, one field
>>   couldn't be represented: the structure contains a union
>>
>>             union fpu_fr
>>             {
>>                 uint[32]        fpu_regs;
>>                 double[32]      fpu_dregs;
>>                 /* long double[16]      fpu_qregs; */
>>
>>   but there's no D type corresponding to long double: the D real type
>>   represents the biggest floating-point type implemented in hardware,
>>   and AFAIK no SPARC CPU ever implemented 128-bit floating point.
>>   Still, with this missing the alignment of the union is wrong.
>>
>
> In practice however, the 'real' type maps to C 'long double', so you
> can safely use real[16] here.

Fine, that's what I did in the updated patch.

Thanks.
	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University