From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_REPLYTO_END_DIGIT, MAILING_LIST_MULTI,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id C4A2A1F4B4 for ; Mon, 10 Aug 2020 21:40:00 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9A8C43861810; Mon, 10 Aug 2020 21:39:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9A8C43861810 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1597095599; bh=sdla2+KoObXu/k3xNTiDIYPB8WFn2e9k8JqqvaubXf0=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=KKsCgXWfLPAC0DMfCD8Hy/t8B/6ZlDxOzxFPn89m8EaUuPFoav8KIxM5CDtNpcNc/ zN4j6mjGmYu5wfbgJPIe8uB8kTypngsRdD9snSwckxJj/UBJwWWbGQZceSslte51uG UnrXSrlhdsgz6/Y3UborEzFTJbevHVhXX5WzZJ88= Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by sourceware.org (Postfix) with ESMTPS id 401B2385040C for ; Mon, 10 Aug 2020 21:39:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 401B2385040C Received: by mail-il1-x144.google.com with SMTP id y18so8834284ilp.10 for ; Mon, 10 Aug 2020 14:39:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=sdla2+KoObXu/k3xNTiDIYPB8WFn2e9k8JqqvaubXf0=; b=ZBC5gE6MIlnLNPAUXZQHIqCFtIF4jpLkEInHFxtQpcsRhc2JR2EnKY/YPh23t12097 fv/LG9dV18CmXkgV8O8PnI/P1Yocthcj+5GJ+5tZoU4e4rPs0C+pYHn/SFR/HMxpyoZa vbO+9zV8msLDmwYws5X89KTM8794PRLyB6KIZOQYrpIF3W9mOMLQLEGv15+ktKgqcz9C MEqWlWF+BzsBUyekzcZuj9tHEF8XO+kxWzPc0i2Ci5gfyBLfw3uBa7OwkHK1NoBjqPJ/ cJRGtvVezg4IEYmRIpEi6/vur0xgUEAVw5+No4RJ4TefC3BLctrLqBCnyoCgZHukfnYu cz+Q== X-Gm-Message-State: AOAM533nftclpeW9pHClhWj7/ZXWPcLxk2uAZAjDCn2kafHV+lwFh6ix PE6vWXurZ84bOrpdd6cETpEBqJZoc68RuMgHHeiA66uGXcA= X-Google-Smtp-Source: ABdhPJxWiWQCCz7iXK9F/Mom+vdo0CaoB2BQ3eIIbKwaXySMWCW5BFyDBC4hvE8Ihg+ZXPYThrBn9WEqpRLUfQ+mk2o= X-Received: by 2002:a05:6e02:13f1:: with SMTP id w17mr20058982ilj.131.1597095596513; Mon, 10 Aug 2020 14:39:56 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: Date: Mon, 10 Aug 2020 14:29:33 -0700 Message-ID: Subject: Re: [PATCH v2 09/18] RISC-V: The ABI implementation for 32-bit To: "Maciej W. Rozycki" Content-Type: text/plain; charset="UTF-8" X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Alistair Francis via Libc-alpha Reply-To: Alistair Francis Cc: GNU C Library , Alistair Francis Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Fri, Jul 10, 2020 at 6:24 PM Maciej W. Rozycki wrote: > > On Fri, 10 Jul 2020, Alistair Francis wrote: > > > > I note there is an extensive discussion on the way to move forward here: > > > > > > 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). > > > > > 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. > > > > 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. > > > > > +# 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