From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 2ED1B38543A6 for ; Thu, 12 Jan 2023 16:50:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2ED1B38543A6 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0246B13D5; Thu, 12 Jan 2023 08:50:52 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F05A73F587; Thu, 12 Jan 2023 08:50:08 -0800 (PST) From: Richard Sandiford To: Jakub Jelinek via Gcc-patches Mail-Followup-To: Jakub Jelinek via Gcc-patches ,Florian Weimer , Jakub Jelinek , Andre Vieira , Andrew Pinski , Jeff Law , richard.sandiford@arm.com Cc: Florian Weimer , Jakub Jelinek , Andre Vieira , Andrew Pinski , Jeff Law Subject: Re: [PATCH] Various fixes for DWARF register size computation References: <87pmbvx41g.fsf@oldenburg.str.redhat.com> <878rijwy0u.fsf@oldenburg.str.redhat.com> Date: Thu, 12 Jan 2023 16:50:07 +0000 In-Reply-To: (Jakub Jelinek via Gcc-patches's message of "Tue, 3 Jan 2023 14:54:10 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-31.6 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Jakub Jelinek via Gcc-patches writes: > On Tue, Jan 03, 2023 at 02:25:21PM +0100, Florian Weimer wrote: >> > Though, I still wonder, because all of this is a hack for a single tar= get >> > - x86_64-linux -m64 - I think no other target has similar constant >> > sizes, >>=20 >> Really? That's odd. > > I've tried about 30 cross compilers I had around, I admit it isn't > exhaustive. > >> Is it because other architectures track callee-saved vector registers >> through unwinding? > > I think it is far more than just vector registers, it can be floating poi= nt > registers, or just one integral or special register of a different size e= tc. > >> > Or, if you want to do it on the compiler side, instead of predefining >> > __LIBGCC_DWARF_REG_SIZES_CONSTANT__ and __LIBGCC_DWARF_REG_MAXIMUM__ >> > register conditionally a new builtin, __builtin_dwarf_reg_size, >> > which would be defined only if -fbuilding-libgcc and the compiler dete= rmines >> > dwarf_reg_size is desirable to be computed inline without a table and >> > would fold the builtin to say that >> > index <=3D 16U ? 8 : 0 on x86_64 -m64, >> > index <=3D 9U ? 4 : index - 11U <=3D 5U ? 12 : 0 on x86_64 -m32 etc. >> > and if the expression is too large/complex, wouldn't predefine the bui= ltin. >>=20 >> I think the pre-computation of the size array is useful even for targets >> where the expression is not so simple, but the array elements are still >> constants. A builtin like __builtin_dwarf_reg_size could use a >> reference to a constant static array, so that we can get rid of the >> array initialization code in libgcc. Before we can do that, we need to > > I think constant static array might be sometimes an option too, but not > always, not just because of AArch64, or other potential poly-int arches > (RISCV?), but also if something could depend on some runtime check. > It is true that most of the time it is constant and depends just on the > target or more often on target + options combo (mostly ABI related option= s). > >> figure out if the fully dynamic register sizes on AArch64 with SVE are >> actually correct=E2=80=94and if we need to fix the non-SVE unwinder to w= ork >> properly for SVE programs. >>=20 >> So I don't want to revert the size array computation just yet. >>=20 >> > Or, is it actually the use of table that is bad on the unwinder side, >> > or lack of a small upper bound for what you get from the table? >> > In that case you could predefine upper bound on the sizes instead (if >> > constant) and simply add if (size > __LIBGCC_DWARF_REG_SIZE_MAX__) >> > __builtin_unreachable ()). >>=20 >> It also matters what kind of register sizes are used in practice. > > Yes, but that is hard to find out easily. E.g. some registers might be > only saved/restored in signal frames and nowhere else, others only rarely > touched but still we'd need their sizes if they are ever used. > >> Should I repost this patch with the three nits fixed? Or should I >> revert two of the three patches I committed instead? > > I lean towards reversion and trying to figure out something that works > on more than one arch. It doesn't have to improve all arches (say > AArch64 is clearly a nightmare that isn't handled correctly even without > any changes - as noted in the PRs, if libgcc is built without SVE, it will > hardcode 8, while if it is built with SVE, it will be runtime dependent a= nd > will be wrong in theory when some HW has 2048 bit SVE vectors - when it is > 256 bytes), but still watching into what we compile -O2 -nostdinc -fbuild= ing-libgcc I'm jumping in here without fully understanding the context, so maybe this is exactly your point, but: the SIMD/FP DWARF registers are supposed to be size 8 regardless of which features are enabled. That's already only half of the hardware register size for base Armv8-A, since Advanced SIMD registe= rs are 16 bytes in size. So yeah, if we're using the hardware register size then something is wrong. Thanks, Richard > static unsigned char dwarf_reg_size_table[__LIBGCC_DWARF_FRAME_REGISTERS_= _+1]; > > void > foo (void) > { > __builtin_init_dwarf_reg_size_table (dwarf_reg_size_table); > } > > on at least 10 most common arches would be useful and optimizing what is > easily possible would be nice. > > Jakub