From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Subject: Re: [PATCH v2 09/18] RISC-V: The ABI implementation for 32-bit
Date: Thu, 27 Aug 2020 16:43:25 -0300 [thread overview]
Message-ID: <b54796ab-2830-8082-a597-a40ee365ddba@linaro.org> (raw)
In-Reply-To: <CAKmqyKPsKswYe4p3UmFWTqbHm-8WvHxat1mPJ9yA8A2zvRc2sQ@mail.gmail.com>
On 10/08/2020 18:29, Alistair Francis via Libc-alpha wrote:
> On Fri, Jul 10, 2020 at 6:24 PM Maciej W. Rozycki <macro@wdc.com> wrote:
>>
>> On Fri, 10 Jul 2020, Alistair Francis wrote:
>>
>>>> I note there is an extensive discussion on the way to move forward here:
>>>> <https://sourceware.org/glibc/wiki/Y2038ProofnessDesign#utmp_types_and_APIs>
>>>> We might as well try to implement it right away, so as to avoid being
>>>> limited to 32-bit time records here.
>>>
>>> Is there an advantage of doing it now or can we put this off for the
>>> next release?
>>
>> The change is major enough it'll have to wait for the next development
>> cycle anyway. It shouldn't matter that much for RV32 glibc deployments
>> though, given the amount of suitable hardware available.
>>
>>>> NB some existing ports do have __WORDSIZE_TIME64_COMPAT32 set and cleared
>>>> for their 64-bit and 32-bit ABIs respectively, as per the note in our
>>>> top-level bits/wordsize.h, however this reflects the state as before we
>>>> introduced the possibility for `__time_t' to be a 64-bit type with
>>>> `__WORDSIZE == 32' ABIs. Given the turn of events I think the note ought
>>>> to be updated accordingly; I gather it was missed with commit 07fe93cd9850
>>>> ("generic/typesizes.h: Add support for 32-bit arches with 64-bit types").
>>
>> Will you be able to look into it?
>>
>> The context here is before Y2038 changes __WORDSIZE_TIME64_COMPAT32 would
>> only be clear for 64-bit ABIs with those 64-bit systems that do not have
>> any 32-bit ABI (compatibility mode) to support, such as the DEC Alpha.
>> And it would always be clear for 32-bit ABIs, so as to use the proper
>> `__time_t' type without changing the width of actual data held there in
>> the structure.
>>
>> I'm not sure what the story is behind the S/390 port though; perhaps it
>> does not support ABI coexistence in a single run-time environment.
>
> I was just about to start this but Adhemerval has very kindly sent a
> patch series already. I'm going to leave this as is (defining it as 1
> for RV32 and RV64).
Don't forget to help on review ;).
>
>
>>
>>>>> int __cur_writer;
>>>>> int __shared;
>>>>> unsigned long int __pad1;
>>>>> unsigned long int __pad2;
>>>>> unsigned int __flags;
>>>>> +#else
>>>>> +# if __BYTE_ORDER == __BIG_ENDIAN
>>>>> + unsigned char __pad1;
>>>>> + unsigned char __pad2;
>>>>> + unsigned char __shared;
>>>>> + unsigned char __flags;
>>>>> +# else
>>>>> + unsigned char __flags;
>>>>> + unsigned char __shared;
>>>>> + unsigned char __pad1;
>>>>> + unsigned char __pad2;
>>>>> +# endif
>>>>> + int __cur_writer;
>>>>> +#endif
>>>>> };
>>>>
>>>> I note with this change the RV32 structure will use the generic layout as
>>>> per sysdeps/nptl/bits/struct_rwlock.h, however regrettably RV64 does not.
>>>> Would it make sense to instead have the layout the same between RV64 and
>>>> RV32, perhaps by redefining `__pad1' and `__pad2' in terms of `unsigned
>>>> long long' (which would have alignment implications though) or otherwise?
>>>
>>> I'm not sure which one is better. On one hand it seems better to be
>>> more generic and therefore RV32 should use the generic interface. On
>>> the other hand the more similar they are the better. I'm still leaning
>>> towards we should be generic where possible.
>>
>> It would be good to get a second opinion here.
>
> The RV64 __pthread_rwlock_arch_t is exactly the same as the AArch64
> version. My guess is that it was just copied to be the same.
>
> The generic version also has this comment
>
> /* Generic struct for both POSIX read-write lock. New ports are expected
> to use the default layout, however archictetures can redefine it to add
> arch-specific extensions (such as lock-elision). The struct have a size
> of 32 bytes on both LP32 and LP64 architectures. */
>
> As we aren't adding any arch-specific extensions I think we should
> keep the generic one. It's unfortunate we didn't do that for RV64, but
> too late now.
>
> The only downside I see in using the generic version is what happens
> if the value of _shared or flags overflows a char. There are currently
> only 4 flags though, so I think it's ok.
I come up with the generic interface by using the already one in place for
mostly 64-bit architecture and the internal layout is mainly to avoid duplicate
the internal type on multiple architectures (so we can use the generic
implementation).
The only usage I can think of where same structure for RV64 and RV32 may
pose a role is to use a shared rwlock between a 32-bit and a 64-bit process
(pthread_rwlockattr_setpshared with PTHREAD_PROCESS_SHARED). But afaik this
is not supported on other architectures.
>
>>
>>>> Is there any benefit from having `__flags' and `__shared' (and the
>>>> reserve) grouped within a single 32-bit word? I gather there is, given
>>>> the lengths gone to to match the bit lanes across the word regardless of
>>>> the endianness. But what is it?
>>>
>>> I have no idea.
>>
>> Especially given this.
The internal member that are most concurrently accessed by the common operations
(pthread_rwlock_{clock,timed}{wr,rd}lock) are the __readers, __writers,
__wrphase_futex, __writers_futex, and __cur_writer. The __shared and __flag
are either accessed with non-atomic operation or at initialization, so I don't
think there is much different in packing or not in a 32-bit word. My
understanding is architectures usually does that to avoid possible internal
padding, since the usual types for both member are uint8_t.
>>
>>>>> +# define _FP_DIV_MEAT_S(R, X, Y) _FP_DIV_MEAT_1_udiv_norm (S, R, X, Y)
>>>>> +# define _FP_DIV_MEAT_D(R, X, Y) _FP_DIV_MEAT_2_udiv (D, R, X, Y)
>>>>> +# define _FP_DIV_MEAT_Q(R, X, Y) _FP_DIV_MEAT_4_udiv (Q, R, X, Y)
>>>>> +
>>>>> +# define _FP_NANFRAC_S _FP_QNANBIT_S
>>>>> +# define _FP_NANFRAC_D _FP_QNANBIT_D, 0
>>>>> +# define _FP_NANFRAC_Q _FP_QNANBIT_Q, 0, 0, 0
>>>>
>>>> Likewise. There seems to be an established practice for this header
>>>> across architectures to have no space between macro arguments or before
>>>> the opening parenthesis. This might help with the alignment.
>>>
>>> I still think it makes sense to follow the glibc style though, even if
>>> other archs don't.
>>>
>>> Let me know if it should be a different way and I'll update it.
>>
>> There is the issue of the discrepancy compared to the libgcc version, and
>> while `diff -l' and `patch -l' solve that for manual processing, more
>> sophisticated tools may not cope and require manual intervention.
>>
>> Again, I would suggest getting a second opinion.
>
> The current option matches the rest of the file, so I think it makes
> sense to leave as is.
>
> If this is a problem we can just change the spacing for the entire
> file in a future patch.
>
> Alistair
>
>>
>> Maciej
next prev parent reply other threads:[~2020-08-27 19:43 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-03 16:25 [PATCH v2 00/18] glibc port for 32-bit RISC-V (RV32) Alistair Francis
2020-06-03 16:25 ` [PATCH v2 01/18] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64 Alistair Francis
2020-07-07 22:06 ` Maciej W. Rozycki
2020-07-10 15:27 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 02/18] RISC-V: Define __NR_* as __NR_*_time64/64 for 32-bit Alistair Francis
2020-07-08 0:09 ` Maciej W. Rozycki
2020-07-08 17:08 ` Adhemerval Zanella
2020-07-09 17:14 ` Alistair Francis
2020-07-16 0:23 ` Maciej W. Rozycki
2020-07-09 17:10 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 03/18] RISC-V: Add support for 32-bit vDSO calls Alistair Francis
2020-07-08 1:01 ` Maciej W. Rozycki
2020-07-08 18:17 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 04/18] RISC-V: Support dynamic loader for the 32-bit Alistair Francis
2020-07-08 1:35 ` Maciej W. Rozycki
2020-06-03 16:25 ` [PATCH v2 05/18] RISC-V: Add path of library directories " Alistair Francis
2020-07-08 18:42 ` Maciej W. Rozycki
2020-07-09 17:03 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 06/18] RISC-V: Add arch-syscall.h for RV32 Alistair Francis
2020-07-08 19:33 ` Maciej W. Rozycki
2020-06-03 16:25 ` [PATCH v2 07/18] RISC-V: nptl: update default pthread-offsets.h Alistair Francis
2020-07-09 0:14 ` Maciej W. Rozycki
2020-07-09 11:47 ` Adhemerval Zanella
2020-07-15 19:23 ` Maciej W. Rozycki
2020-08-10 17:34 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 08/18] riscv32: Add an architecture ipctypes.h Alistair Francis
2020-07-09 2:46 ` Maciej W. Rozycki
2020-07-09 11:36 ` Adhemerval Zanella
2020-06-03 16:25 ` [PATCH v2 09/18] RISC-V: The ABI implementation for 32-bit Alistair Francis
2020-07-09 23:33 ` Maciej W. Rozycki
2020-07-10 16:45 ` Alistair Francis
2020-07-11 1:24 ` Maciej W. Rozycki
2020-08-10 21:29 ` Alistair Francis
2020-08-27 19:43 ` Adhemerval Zanella [this message]
2020-09-25 23:03 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 10/18] RISC-V: Hard float support " Alistair Francis
2020-07-11 0:49 ` Maciej W. Rozycki
2020-07-11 15:49 ` Alistair Francis
2020-07-11 22:13 ` Maciej W. Rozycki
2020-07-12 15:34 ` Alistair Francis
2020-07-12 22:10 ` Maciej W. Rozycki
2020-08-27 18:36 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 11/18] RISC-V: Add ABI lists Alistair Francis
2020-07-12 20:54 ` Maciej W. Rozycki
2020-07-13 16:14 ` Alistair Francis
2020-06-03 16:25 ` [PATCH v2 12/18] RISC-V: Add the RV32 libm-test-ulps Alistair Francis
2020-06-03 17:34 ` Joseph Myers
2020-06-05 4:01 ` Alistair Francis
2020-06-03 16:26 ` [PATCH v2 13/18] RISC-V: Fix llrint and llround missing exceptions on RV32 Alistair Francis
2020-06-03 16:26 ` [PATCH v2 14/18] RISC-V: Build Infastructure for 32-bit Alistair Francis
2020-06-03 16:26 ` [PATCH v2 15/18] riscv32: Specify the arch_minimum_kernel as 5.4 Alistair Francis
2020-06-03 16:26 ` [PATCH v2 16/18] RISC-V: Add rv32 path to RTLDLIST in ldd Alistair Francis
2020-06-03 16:26 ` [PATCH v2 17/18] Documentation for the RISC-V 32-bit port Alistair Francis
2020-06-03 16:26 ` [PATCH v2 18/18] Add RISC-V 32-bit target to build-many-glibcs.py Alistair Francis
2020-06-15 22:37 ` [PATCH v2 00/18] glibc port for 32-bit RISC-V (RV32) Alistair Francis
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=b54796ab-2830-8082-a597-a40ee365ddba@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
/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).