diff mbox series

[RFC,12/12] C11 thrd: Downgrade the default alignment of mtx_t

Message ID 20230212111044.610942-13-bugaevc@gmail.com
State Rejected, archived
Headers
Series Towards glibc on x86_64-gnu |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch series failed to apply

Commit Message

Sergey Bugaev Feb. 12, 2023, 11:10 a.m. UTC 
  ..so that it can match the alignment of pthread_mutex_t on x86_64-gnu.
This likely breaks many other arches (if not all of them), though.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/pthread/threads.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Samuel Thibault Feb. 12, 2023, 3:18 p.m. UTC | #1 
Sergey Bugaev, le dim. 12 févr. 2023 14:10:43 +0300, a ecrit:
> ..so that it can match the alignment of pthread_mutex_t on x86_64-gnu.

I'd say rather make pthread_mutex_t aligned on long int, so we can
possibly in the future put some pointers in it without breaking the ABI.

> This likely breaks many other arches (if not all of them), though.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/pthread/threads.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/pthread/threads.h b/sysdeps/pthread/threads.h
> index 860d597d..207c1dee 100644
> --- a/sysdeps/pthread/threads.h
> +++ b/sysdeps/pthread/threads.h
> @@ -64,7 +64,7 @@ typedef __once_flag once_flag;
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEX_T];
> -  long int __align __LOCK_ALIGNMENT;
> +  int __align __LOCK_ALIGNMENT;
>  } mtx_t;
>  
>  typedef union
> -- 
> 2.39.1
Sergey Bugaev Feb. 12, 2023, 3:52 p.m. UTC | #2 
On Sun, Feb 12, 2023 at 6:18 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> I'd say rather make pthread_mutex_t aligned on long int, so we can
> possibly in the future put some pointers in it without breaking the ABI.

I honestly have 0 idea how it is not 8-aligned right now (nor how its
size is 32 and not like 56), given that this is its definition...

no, strike that, I see that there are *two* versions of
struct___pthread_mutex.h, one in sysdeps/mach/hurd/htl/bits and the
other one in sysdeps/htl/bits/types, and the former one wins. The
latter one seems unused then, its only point was to confuse the hell
out of me.

Sergey
Samuel Thibault Feb. 12, 2023, 4:29 p.m. UTC | #3 
Sergey Bugaev, le dim. 12 févr. 2023 18:52:37 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 6:18 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > I'd say rather make pthread_mutex_t aligned on long int, so we can
> > possibly in the future put some pointers in it without breaking the ABI.
> 
> I honestly have 0 idea how it is not 8-aligned right now (nor how its
> size is 32 and not like 56), given that this is its definition...
> 
> no, strike that, I see that there are *two* versions of
> struct___pthread_mutex.h, one in sysdeps/mach/hurd/htl/bits and the
> other one in sysdeps/htl/bits/types, and the former one wins. The
> latter one seems unused then, its only point was to confuse the hell
> out of me.

History is full of remnants. That's why one just has to take the time to
clean things up and avoid leaving things behind oneself.  It seems the
a99155555c21 cleanup missed this one.

Samuel
diff mbox series

Patch

diff --git a/sysdeps/pthread/threads.h b/sysdeps/pthread/threads.h
index 860d597d..207c1dee 100644
--- a/sysdeps/pthread/threads.h
+++ b/sysdeps/pthread/threads.h
@@ -64,7 +64,7 @@  typedef __once_flag once_flag;
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEX_T];
-  long int __align __LOCK_ALIGNMENT;
+  int __align __LOCK_ALIGNMENT;
 } mtx_t;
 
 typedef union