From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 72AF23858D1E for ; Tue, 3 Jan 2023 13:25:28 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1672752328; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=D+Pr/Y/A0ur2nT/XPzqipUff5yWh4PWhaLYT29cX/pM=; b=ANiDK336zW3qqI7SmwHcoPPoUz4vTfdcdcK0akA4baKA+Dvu6sfUMNP+saYxyaJ5Celugi VOuO2dSx8KYM0e/B7A4kyVeh5BAvznMhVy+ZjbskhD3nb1jrEjSVah/sLjUiP+BeJugltY 8KIKhZd3r/nST+8DXkcvtU2LtrHYHH8= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-133-f8p97MZaOXGmkBCqDrzuzQ-1; Tue, 03 Jan 2023 08:25:25 -0500 X-MC-Unique: f8p97MZaOXGmkBCqDrzuzQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9CA853C10EC8; Tue, 3 Jan 2023 13:25:24 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.92]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2E12A492C14; Tue, 3 Jan 2023 13:25:23 +0000 (UTC) From: Florian Weimer To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org, Andre Vieira , Andrew Pinski , Jeff Law Subject: Re: [PATCH] Various fixes for DWARF register size computation References: <87pmbvx41g.fsf@oldenburg.str.redhat.com> Date: Tue, 03 Jan 2023 14:25:21 +0100 In-Reply-To: (Jakub Jelinek's message of "Tue, 3 Jan 2023 13:05:32 +0100") Message-ID: <878rijwy0u.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham 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: > On Tue, Jan 03, 2023 at 12:15:23PM +0100, Florian Weimer wrote: >> --- a/gcc/debug.h >> +++ b/gcc/debug.h >> @@ -245,7 +245,18 @@ extern const struct gcc_debug_hooks vmsdbg_debug_ho= oks; >> =20 >> /* Dwarf2 frame information. */ >> =20 >> -extern int dwarf_reg_sizes_constant (); >> +/* Query size information about DWARF registers. */ >> +struct dwarf_single_register_size >> +{ >> + dwarf_single_register_size(); > > Formatting, space before ( > >> @@ -334,27 +333,39 @@ generate_dwarf_reg_sizes (poly_uint16 *sizes) >> targetm.init_dwarf_reg_sizes_extra (sizes); >> } >> =20 >> -/* Return 0 if the DWARF register sizes are not constant, otherwise >> - return the size constant. */ >> - >> -int >> -dwarf_reg_sizes_constant () >> +dwarf_single_register_size::dwarf_single_register_size() > > Likewise. > >> + for (int i =3D DWARF_FRAME_REGISTERS; i >=3D 0; --i) >> + { >> + unsigned short value; >> + if (!sizes[i].is_constant (&value) || value !=3D 0) > > if (!known_eq (sizes[i], 0)) > ? Right. > Though, I still wonder, because all of this is a hack for a single target > - x86_64-linux -m64 - I think no other target has similar constant > sizes, Really? That's odd. Is it because other architectures track callee-saved vector registers through unwinding? > whether > it wouldn't be better to revert all this compiler side stuff and handle i= t > purely on the libgcc side - allow target headers to specify a simple > expression how to compute dwarf_reg_size + don't define dwarf_reg_size_ta= ble > array in that case and instead in a testcase verify that > __builtin_init_dwarf_reg_size_table initializes an array to the exact sam= e > values as the libgcc/config/**/*.h overridden dwarf_reg_size version. > That way, for x86_64-linux we can use > ((index) <=3D __LIBGCC_DWARF_FRAME_REGISTERS__ ? 8 : 0) > but could provide something reasonable even for other targets if it impro= ves > the unwinder sufficiently. > Say s390x-linux -m64 is > ((index) <=3D 32 ? 8 : (index) =3D=3D 33 ? 4 : 0) > etc. > 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 determi= nes > 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 builti= n. 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 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 work properly for SVE programs. So I don't want to revert the size array computation just yet. > 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 ()). It also matters what kind of register sizes are used in practice. Looking at the FDE for _Unwind_RaiseException on i686, we only save 4-byte registers there, I think. Perhaps only non-GCC-generated code may exercise the other register sizes? That's different on AArch64, where the vector registers are saved as well. Should I repost this patch with the three nits fixed? Or should I revert two of the three patches I committed instead? Thanks, Florian