public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Iain Buclaw <ibuclaw@gdcproject.org>
To: ro@cebitec.uni-bielefeld.de
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: Fix D compilation on Solaris
Date: Wed, 31 Oct 2018 17:51:00 -0000	[thread overview]
Message-ID: <CABOHX+cwWEKjo7DR2O8PSQ9G_Y7YDj8GzQTeoiZhprRnRqhuEQ@mail.gmail.com> (raw)
In-Reply-To: <yddo9bazc27.fsf@CeBiTec.Uni-Bielefeld.DE>

On Wed, 31 Oct 2018 at 10:43, 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 :-)
>
> and now with the updated patch ;-)
>

Thanks, the front-end and library parts should be posted upstream.

Mapping would be:
- d/dmd: https://github.com/dlang/dmd/tree/dmd-cxx
- libdruntime/core: https://github.com/dlang/druntime
- libphobos/src/std: https://github.com/dlang/phobos

I can take care of this, then backport/merge it down here.

As for the patch itself:

> --- a/gcc/config/default-d.c
> +++ b/gcc/config/default-d.c
> @@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> +#include "memmodel.h"
>  #include "tm_d.h"
>  #include "d/d-target.h"
>  #include "d/d-target-def.h"

Is this still required?  For sure it would cover non-glibc,
non-solaris sparc targets though.

> diff --git a/gcc/config/sol2-d.c b/gcc/config/sol2-d.c
> new file mode 100644
> --- /dev/null
> +++ b/gcc/config/sol2-d.c

[-- snip --]

> +solaris_d_os_builtins (void)
> +{
> +  d_add_builtin_version ("Posix");
> +  d_add_builtin_version ("Solaris"); \
> +}
> +

I'll assume that backslash is a typo.

You'll also need to add this target hook:

    /* Implement TARGET_D_CRITSEC_SIZE for Solaris targets.  */

    static unsigned
    solaris_d_critsec_size (void)
    {
      /* This is the sizeof pthread_mutex_t.  */
      return 24;
    }

I hope that pthread_mutex_t does not differ between x86 and SPARC.

> diff --git a/gcc/config/t-sol2 b/gcc/config/t-sol2
> --- a/gcc/config/t-sol2
> +++ b/gcc/config/t-sol2
> @@ -16,7 +16,7 @@
>  # along with GCC; see the file COPYING3.  If not see
>  # <http://www.gnu.org/licenses/>.
>
> -# Solaris-specific format checking and pragmas
> +# Solaris-specific format checking and pragmas.
>  sol2-c.o: $(srcdir)/config/sol2-c.c
>  $(COMPILE) $<
>  $(POSTCOMPILE)

Not sure what the policy is about mixing unrelated changes in a patch here.

> diff --git a/libphobos/libdruntime/core/sys/posix/ucontext.d b/libphobos/libdruntime/core/sys/posix/ucontext.d
> --- a/libphobos/libdruntime/core/sys/posix/ucontext.d
> +++ b/libphobos/libdruntime/core/sys/posix/ucontext.d

[-- snip --]

> + struct fq
> + {
> +     union FQu
> +     {
> + double whole;
> + _fpq fpq;
> +     };
> + }

Just an FYI, this won't do what I think you expect, 'struct fq' here
would be an empty struct.  Better make this an anonymous union, I can
see the same mistake done elsewhere.

struct fq
{
    union
    {
        double whole;
        _fpq fpq;
    }
}

> diff --git a/libphobos/libdruntime/core/thread.d b/libphobos/libdruntime/core/thread.d
> --- a/libphobos/libdruntime/core/thread.d
> +++ b/libphobos/libdruntime/core/thread.d
> @@ -1547,7 +1547,7 @@ private:
>
>      version (Solaris)
>      {
> -        __gshared immutable bool m_isRTClass;
> +        __gshared bool m_isRTClass;
>      }
>
>  private:

This is curious, I wonder when was the last time someone tested x86
Solaris in upstream.  What was the compilation error?

> diff --git a/libphobos/libdruntime/rt/sections_solaris.d b/libphobos/libdruntime/rt/sections_solaris.d
> --- a/libphobos/libdruntime/rt/sections_solaris.d
> +++ b/libphobos/libdruntime/rt/sections_solaris.d

[-- snip --]

>
> +// interface for core.thread to inherit loaded libraries
> +void* pinLoadedLibraries() nothrow @nogc
> +{
> +    return null;
> +}
> +

I ran into this on OSX as well.  The problem lies elsewhere, and I'm
going to fix this.  The shared library is being built with
'-fversion=Shared', but this is only valid for targets that use
rt/sections_elf_shared.d

I suspect that Solaris/SPARC should be using the ELF sections support anyway.

Apart from comments, the GCC parts look OK to me.  Unless you want to
raise a pull request, I'll submit the library patches upstream later.

Thanks!
Iain

  reply	other threads:[~2018-10-31 17:34 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-30 12:18 Rainer Orth
2018-10-30 14:49 ` Rainer Orth
2018-10-30 19:00   ` Iain Buclaw
2018-10-31 10:02     ` Rainer Orth
2018-10-31 10:03       ` Rainer Orth
2018-10-31 17:51         ` Iain Buclaw [this message]
2018-11-04 16:47           ` Rainer Orth
2018-10-31 17:33       ` Iain Buclaw
2018-11-04 16:47         ` Rainer Orth
2018-11-03 22:23       ` Iain Buclaw
2018-11-04  0:08         ` Iain Buclaw
2018-11-04 16:50           ` Rainer Orth
2018-11-04 18:31             ` Iain Buclaw
2018-11-04 16:47         ` Rainer Orth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABOHX+cwWEKjo7DR2O8PSQ9G_Y7YDj8GzQTeoiZhprRnRqhuEQ@mail.gmail.com \
    --to=ibuclaw@gdcproject.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=ro@cebitec.uni-bielefeld.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).