[netbsd] Define TARGET_D_CRITSEC_SIZE for D language

Message ID CABOHX+dTtGXn+8ukwhyzZCMye0CRdKGNtRzcfwQDBJL3d3rfhw@mail.gmail.com
State New
Headers show
Series
  • [netbsd] Define TARGET_D_CRITSEC_SIZE for D language
Related show

Commit Message

Iain Buclaw April 23, 2019, 11:13 p.m.
Hi,

This patch adds missing implementation of TARGET_D_CRITSEC_SIZE, which
would be noticed when using any bare synchronized statements.

I couldn't see any target-specific alternatives of pthread_mutex_t in
netbsd headers, so the condition should be right.

OK for trunk?

-- 
Iain
---
gcc/ChangeLog:

2019-04-24  Iain Buclaw  <ibuclaw@gdcproject.org>

        * config/netbsd-d.c (netbsd_d_critsec_size): New function.
        (TARGET_D_CRITSEC_SIZE): Define as netbsd_d_critsec_size.

---

Comments

Kamil Rytarowski April 23, 2019, 11:56 p.m. | #1
On 24.04.2019 01:13, Iain Buclaw wrote:
> Hi,

> 

> This patch adds missing implementation of TARGET_D_CRITSEC_SIZE, which

> would be noticed when using any bare synchronized statements.

> 

> I couldn't see any target-specific alternatives of pthread_mutex_t in

> netbsd headers, so the condition should be right.

> 

> OK for trunk?

> 


This patch is wrong.

sizeof(pthread_mutex_t) depends on CPU.

Just check that __cpu_simple_lock_nv_t that can be char, int, struct.
Iain Buclaw April 24, 2019, 1:30 a.m. | #2
On Wed, 24 Apr 2019 at 01:56, Kamil Rytarowski <n54@gmx.com> wrote:
>

> On 24.04.2019 01:13, Iain Buclaw wrote:

> > Hi,

> >

> > This patch adds missing implementation of TARGET_D_CRITSEC_SIZE, which

> > would be noticed when using any bare synchronized statements.

> >

> > I couldn't see any target-specific alternatives of pthread_mutex_t in

> > netbsd headers, so the condition should be right.

> >

> > OK for trunk?

> >

>

> This patch is wrong.

>

> sizeof(pthread_mutex_t) depends on CPU.

>

> Just check that __cpu_simple_lock_nv_t that can be char, int, struct.

>


Ah, thanks for pointing to that.  I've made a small working example,
and it looks like only three have a different size, aarch64, hppa, and
hppa64.

https://explore.dgnu.org/z/U29cni

I'll add special handling for them, but otherwise 48/28 looks like the
reasonable default.

-- 
Iain
Kamil Rytarowski April 24, 2019, 11:04 a.m. | #3
On 24.04.2019 03:30, Iain Buclaw wrote:
> On Wed, 24 Apr 2019 at 01:56, Kamil Rytarowski <n54@gmx.com> wrote:

>>

>> On 24.04.2019 01:13, Iain Buclaw wrote:

>>> Hi,

>>>

>>> This patch adds missing implementation of TARGET_D_CRITSEC_SIZE, which

>>> would be noticed when using any bare synchronized statements.

>>>

>>> I couldn't see any target-specific alternatives of pthread_mutex_t in

>>> netbsd headers, so the condition should be right.

>>>

>>> OK for trunk?

>>>

>>

>> This patch is wrong.

>>

>> sizeof(pthread_mutex_t) depends on CPU.

>>

>> Just check that __cpu_simple_lock_nv_t that can be char, int, struct.

>>

> 

> Ah, thanks for pointing to that.  I've made a small working example,

> and it looks like only three have a different size, aarch64, hppa, and

> hppa64.

> 

> https://explore.dgnu.org/z/U29cni

> 

> I'll add special handling for them, but otherwise 48/28 looks like the

> reasonable default.

> 


I recommend to solve it differently: autodetect the size during
./configure. I'm not sure that it is still correct.

The sizes can also change over time. There is coming refactoring of
libpthread(3) in NetBSD that can change sizes of these types, at least
on some platforms.
Iain Buclaw April 24, 2019, 11:25 a.m. | #4
On Wed, 24 Apr 2019 at 13:03, Kamil Rytarowski <n54@gmx.com> wrote:
>

> On 24.04.2019 03:30, Iain Buclaw wrote:

> > On Wed, 24 Apr 2019 at 01:56, Kamil Rytarowski <n54@gmx.com> wrote:

> >>

> >> On 24.04.2019 01:13, Iain Buclaw wrote:

> >>> Hi,

> >>>

> >>> This patch adds missing implementation of TARGET_D_CRITSEC_SIZE, which

> >>> would be noticed when using any bare synchronized statements.

> >>>

> >>> I couldn't see any target-specific alternatives of pthread_mutex_t in

> >>> netbsd headers, so the condition should be right.

> >>>

> >>> OK for trunk?

> >>>

> >>

> >> This patch is wrong.

> >>

> >> sizeof(pthread_mutex_t) depends on CPU.

> >>

> >> Just check that __cpu_simple_lock_nv_t that can be char, int, struct.

> >>

> >

> > Ah, thanks for pointing to that.  I've made a small working example,

> > and it looks like only three have a different size, aarch64, hppa, and

> > hppa64.

> >

> > https://explore.dgnu.org/z/U29cni

> >

> > I'll add special handling for them, but otherwise 48/28 looks like the

> > reasonable default.

> >

>

> I recommend to solve it differently: autodetect the size during

> ./configure. I'm not sure that it is still correct.

>


That would not work when building crosses.

> The sizes can also change over time. There is coming refactoring of

> libpthread(3) in NetBSD that can change sizes of these types, at least

> on some platforms.

>


Maybe explaining the intended use better might help.  This is only for
the following lowering done in the front-end (my upstream):

    synchronized { var = 0; }

Loosely converted into equivalent C.

    static char __critsec64[48];
    _d_criticalenter(& __critsec64);
    var = 0;
    _d_criticalexit(& __critsec64);

So long as the statically allocated pthread_mutex_t is big enough,
there should be no problems.

-- 
Iain
Kamil Rytarowski April 24, 2019, 1:29 p.m. | #5
On 24.04.2019 13:25, Iain Buclaw wrote:
> On Wed, 24 Apr 2019 at 13:03, Kamil Rytarowski <n54@gmx.com> wrote:

>>

>> On 24.04.2019 03:30, Iain Buclaw wrote:

>>> On Wed, 24 Apr 2019 at 01:56, Kamil Rytarowski <n54@gmx.com> wrote:

>>>>

>>>> On 24.04.2019 01:13, Iain Buclaw wrote:

>>> https://explore.dgnu.org/z/U29cni

>>>

>>> I'll add special handling for them, but otherwise 48/28 looks like the

>>> reasonable default.

>>>

>>


OK, so please go for this.
Iain Buclaw April 25, 2019, 9:43 a.m. | #6
On Wed, 24 Apr 2019 at 15:28, Kamil Rytarowski <n54@gmx.com> wrote:
>

> On 24.04.2019 13:25, Iain Buclaw wrote:

> > On Wed, 24 Apr 2019 at 13:03, Kamil Rytarowski <n54@gmx.com> wrote:

> >>

> >> On 24.04.2019 03:30, Iain Buclaw wrote:

> >>> On Wed, 24 Apr 2019 at 01:56, Kamil Rytarowski <n54@gmx.com> wrote:

> >>>>

> >>>> On 24.04.2019 01:13, Iain Buclaw wrote:

> >>> https://explore.dgnu.org/z/U29cni

> >>>

> >>> I'll add special handling for them, but otherwise 48/28 looks like the

> >>> reasonable default.

> >>>

> >>

>

> OK, so please go for this.

>


On inspection, it looks like there's no current configuration handling
of aarch64-netbsd, hppa-netbsd, nor hppa64-netbsd.

I've updated the patch to add them, and I've tested with
config-list.mk LIST="aarch64-netbsd hppa-netbsd hppa64-netbsd",
however I don't think that the changes to config.gcc should be
included unless it has been ported proper.

-- 
Iain
---
gcc/ChangeLog:

2019-04-25  Iain Buclaw  <ibuclaw@gdcproject.org>

        * config.gcc (aarch64*-*-netbsd*, hppa*64*-*-netbsd*,
        hppa*-*-netbsd*): Add configurations.
        * config/aarch64/aarch64-netbsd.h: New file.
        * config/netbsd-d.c (netbsd_d_critsec_size): New function.
        (TARGET_D_CRITSEC_SIZE): Define as netbsd_d_critsec_size.
        * config/pa/pa-netbsd.h: New file.

---
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 09fb9ecd2cd..c2d77d60efc 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -1050,6 +1050,10 @@ aarch64*-*-linux*)
 	done
 	TM_MULTILIB_CONFIG=`echo $TM_MULTILIB_CONFIG | sed 's/^,//'`
 	;;
+aarch64*-*-netbsd*)
+	tm_file="${tm_file} elfos.h aarch64/aarch64-elf.h aarch64/aarch64-netbsd.h"
+	tmake_file="${tmake_file} aarch64/t-aarch64"
+	;;
 alpha*-*-linux*)
 	tm_file="elfos.h ${tm_file} alpha/elf.h alpha/linux.h alpha/linux-elf.h glibc-stdint.h"
 	tmake_file="${tmake_file} alpha/t-linux alpha/t-alpha"
@@ -1476,6 +1480,16 @@ hppa*-*-linux*)
 	tmake_file="${tmake_file} pa/t-pa pa/t-linux"
 	d_target_objs="${d_target_objs} pa-d.o"
 	;;
+hppa*64*-*-netbsd*)
+	tm_file="pa/pa64-start.h ${tm_file} elfos.h pa/pa-netbsd.h pa/pa32-regs.h"
+	tmake_file="${tmake_file} pa/t-pa"
+	d_target_objs="${d_target_objs} pa-d.o"
+	;;
+hppa*-*-netbsd*)
+	tm_file="${tm_file} elfos.h pa/pa-netbsd.h pa/pa32-regs.h"
+	tmake_file="${tmake_file} pa/t-pa"
+	d_target_objs="${d_target_objs} pa-d.o"
+	;;
 hppa*-*-openbsd*)
 	target_cpu_default="MASK_PA_11"
 	tm_file="${tm_file} dbxelf.h elfos.h openbsd.h openbsd-stdint.h openbsd-libpthread.h \
diff --git a/gcc/config/aarch64/aarch64-netbsd.h b/gcc/config/aarch64/aarch64-netbsd.h
new file mode 100644
index 00000000000..749dadaddec
--- /dev/null
+++ b/gcc/config/aarch64/aarch64-netbsd.h
@@ -0,0 +1,25 @@
+/* Definitions for AArch64 running NetBSD
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   GCC is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_AARCH64_NETBSD_H
+#define GCC_AARCH64_NETBSD_H
+
+#define NETBSD_TARGET_D_CRITSEC_SIZE 40
+
+#endif  /* GCC_AARCH64_NETBSD_H */
diff --git a/gcc/config/netbsd-d.c b/gcc/config/netbsd-d.c
index 76342aacae3..90e87d04012 100644
--- a/gcc/config/netbsd-d.c
+++ b/gcc/config/netbsd-d.c
@@ -28,6 +28,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "d/d-target.h"
 #include "d/d-target-def.h"
 
+/* Implement TARGET_D_OS_VERSIONS for NetBSD targets.  */
+
 static void
 netbsd_d_os_builtins (void)
 {
@@ -35,7 +37,23 @@ netbsd_d_os_builtins (void)
   d_add_builtin_version ("NetBSD");
 }
 
+/* Implement TARGET_D_CRITSEC_SIZE for NetBSD targets.  */
+
+static unsigned
+netbsd_d_critsec_size (void)
+{
+  /* This is the sizeof pthread_mutex_t.  */
+#ifdef NETBSD_TARGET_D_CRITSEC_SIZE
+  return NETBSD_TARGET_D_CRITSEC_SIZE;
+#else
+  return (POINTER_SIZE == 64) ? 48 : 28;
+#endif
+}
+
 #undef TARGET_D_OS_VERSIONS
 #define TARGET_D_OS_VERSIONS netbsd_d_os_builtins
 
+#undef TARGET_D_CRITSEC_SIZE
+#define TARGET_D_CRITSEC_SIZE netbsd_d_critsec_size
+
 struct gcc_targetdm targetdm = TARGETDM_INITIALIZER;
diff --git a/gcc/config/pa/pa-netbsd.h b/gcc/config/pa/pa-netbsd.h
new file mode 100644
index 00000000000..11de05452d9
--- /dev/null
+++ b/gcc/config/pa/pa-netbsd.h
@@ -0,0 +1,25 @@
+/* Definitions for PA_RISC running NetBSD
+   Copyright (C) 2019 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_AARCH64_NETBSD_H
+#define GCC_AARCH64_NETBSD_H
+
+#define NETBSD_TARGET_D_CRITSEC_SIZE (TARGET_64BIT ? 104 : 52)
+
+#endif  /* GCC_AARCH64_NETBSD_H */

Patch

diff --git a/gcc/config/netbsd-d.c b/gcc/config/netbsd-d.c
index 76342aacae3..c49366dc23b 100644
--- a/gcc/config/netbsd-d.c
+++ b/gcc/config/netbsd-d.c
@@ -28,6 +28,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "d/d-target.h"
 #include "d/d-target-def.h"
 
+/* Implement TARGET_D_OS_VERSIONS for NetBSD targets.  */
+
 static void
 netbsd_d_os_builtins (void)
 {
@@ -35,7 +37,19 @@  netbsd_d_os_builtins (void)
   d_add_builtin_version ("NetBSD");
 }
 
+/* Implement TARGET_D_CRITSEC_SIZE for NetBSD targets.  */
+
+static unsigned
+netbsd_d_critsec_size (void)
+{
+  /* This is the sizeof pthread_mutex_t.  */
+  return (POINTER_SIZE == 64) ? 48 : 28;
+}
+
 #undef TARGET_D_OS_VERSIONS
 #define TARGET_D_OS_VERSIONS netbsd_d_os_builtins
 
+#undef TARGET_D_CRITSEC_SIZE
+#define TARGET_D_CRITSEC_SIZE netbsd_d_critsec_size
+
 struct gcc_targetdm targetdm = TARGETDM_INITIALIZER;