Message ID | 20230214110049.649769-1-felix.willgerodt@intel.com |
---|---|
State | Committed |
Commit | ecbc5c4f9059dd483cf8b0705cbd4e2e9606aad5 |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 34E843858C39 for <patchwork@sourceware.org>; Tue, 14 Feb 2023 11:01:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 34E843858C39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1676372496; bh=MKqERauzahlbqrN53IHq1DtSeZow4YolrsgBjQCbmwE=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=OFJL4Ub8bqYOQJUyTe6vfeKK90gaVU5k3mbvDv6yKQJexUN7x/pNoKlINDInjjyfa AD71V9IhARiGiHbkN9I1rV7Qdy91qLWrNK6/YkGv+qBo3i4GcG2DJnY2uYkT1tVK+i NOi1yAruGG02+n0oNIdFR6l32LpNDD/qmzp8qszY= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by sourceware.org (Postfix) with ESMTPS id 16EC03858D33 for <gdb-patches@sourceware.org>; Tue, 14 Feb 2023 11:01:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 16EC03858D33 X-IronPort-AV: E=McAfee;i="6500,9779,10620"; a="417353084" X-IronPort-AV: E=Sophos;i="5.97,296,1669104000"; d="scan'208";a="417353084" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Feb 2023 03:01:08 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10620"; a="778279214" X-IronPort-AV: E=Sophos;i="5.97,296,1669104000"; d="scan'208";a="778279214" Received: from mulfelix.iul.intel.com (HELO localhost) ([172.28.49.163]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Feb 2023 03:01:06 -0800 To: gdb-patches@sourceware.org Cc: Felix Willgerodt <felix.willgerodt@intel.com> Subject: [PATCH 1/1] gdb, fortran: Fix quad floating-point type for ifort compiler. Date: Tue, 14 Feb 2023 12:00:49 +0100 Message-Id: <20230214110049.649769-1-felix.willgerodt@intel.com> X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Felix Willgerodt <felix.willgerodt@intel.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[1/1] gdb, fortran: Fix quad floating-point type for ifort compiler.
|
|
Commit Message
Willgerodt, Felix
Feb. 14, 2023, 11 a.m. UTC
I fixed this a while ago for ifx, one of the two Intel compilers, in 8d624a9d8050ca96e154215c7858ac5c2d8b0b19. Apparently I missed that the older ifort Intel compiler actually emits slightly different debug info again: 0x0000007a: DW_TAG_base_type DW_AT_byte_size (0x20) DW_AT_encoding (DW_ATE_complex_float) DW_AT_name ("COMPLEX(16)") 0x00000081: DW_TAG_base_type DW_AT_byte_size (0x10) DW_AT_encoding (DW_ATE_float) DW_AT_name ("REAL(16)") This fixes two failures in gdb.fortran/complex.exp with ifort. --- gdb/i386-tdep.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Comments
>>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:
Felix> I fixed this a while ago for ifx, one of the two Intel compilers, in
Felix> 8d624a9d8050ca96e154215c7858ac5c2d8b0b19.
Felix> Apparently I missed that the older ifort Intel compiler actually emits
Felix> slightly different debug info again:
Thank you, this is ok.
Approved-By: Tom Tromey <tom@tromey.com>
Felix> @@ -8238,12 +8238,14 @@ i386_floatformat_for_type (struct gdbarch *gdbarch,
Felix> || strcmp (name, "_Float128") == 0
Felix> || strcmp (name, "complex _Float128") == 0
Felix> || strcmp (name, "complex(kind=16)") == 0
Felix> + || strcmp (name, "COMPLEX(16)") == 0
Felix> || strcmp (name, "complex*32") == 0
Felix> || strcmp (name, "COMPLEX*32") == 0
Felix> || strcmp (name, "quad complex") == 0
Felix> || strcmp (name, "real(kind=16)") == 0
Felix> || strcmp (name, "real*16") == 0
Felix> - || strcmp (name, "REAL*16") == 0)
Felix> + || strcmp (name, "REAL*16") == 0
Felix> + || strcmp (name, "REAL(16)") == 0)
Felix> return floatformats_ieee_quad;
Not your fault but this whole approach seems bad.
Tom
> -----Original Message----- > From: Tom Tromey <tom@tromey.com> > Sent: Dienstag, 14. Februar 2023 16:14 > To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com> > Subject: Re: [PATCH 1/1] gdb, fortran: Fix quad floating-point type for ifort > compiler. > > >>>>> "Felix" == Felix Willgerodt via Gdb-patches <gdb- > patches@sourceware.org> writes: > > Felix> I fixed this a while ago for ifx, one of the two Intel compilers, in > Felix> 8d624a9d8050ca96e154215c7858ac5c2d8b0b19. > > Felix> Apparently I missed that the older ifort Intel compiler actually emits > Felix> slightly different debug info again: > > Thank you, this is ok. > > Approved-By: Tom Tromey <tom@tromey.com> Thanks, I pushed this now. > Felix> @@ -8238,12 +8238,14 @@ i386_floatformat_for_type (struct gdbarch > *gdbarch, > Felix> || strcmp (name, "_Float128") == 0 > Felix> || strcmp (name, "complex _Float128") == 0 > Felix> || strcmp (name, "complex(kind=16)") == 0 > Felix> + || strcmp (name, "COMPLEX(16)") == 0 > Felix> || strcmp (name, "complex*32") == 0 > Felix> || strcmp (name, "COMPLEX*32") == 0 > Felix> || strcmp (name, "quad complex") == 0 > Felix> || strcmp (name, "real(kind=16)") == 0 > Felix> || strcmp (name, "real*16") == 0 > Felix> - || strcmp (name, "REAL*16") == 0) > Felix> + || strcmp (name, "REAL*16") == 0 > Felix> + || strcmp (name, "REAL(16)") == 0) > Felix> return floatformats_ieee_quad; > > Not your fault but this whole approach seems bad. Agreed. But can we change anything really? Felix Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928
>>>>> "Felix" == Willgerodt, Felix via Gdb-patches <gdb-patches@sourceware.org> writes: Felix> || strcmp (name, "_Float128") == 0 Felix> || strcmp (name, "complex _Float128") == 0 Felix> || strcmp (name, "complex(kind=16)") == 0 Felix> + || strcmp (name, "COMPLEX(16)") == 0 Felix> || strcmp (name, "complex*32") == 0 Felix> || strcmp (name, "COMPLEX*32") == 0 Felix> || strcmp (name, "quad complex") == 0 Felix> || strcmp (name, "real(kind=16)") == 0 Felix> || strcmp (name, "real*16") == 0 Felix> - || strcmp (name, "REAL*16") == 0) Felix> + || strcmp (name, "REAL*16") == 0 Felix> + || strcmp (name, "REAL(16)") == 0) Felix> return floatformats_ieee_quad; >> Not your fault but this whole approach seems bad. Felix> Agreed. But can we change anything really? I suppose ideally what we would need is extra information in the DWARF. Although... i386_floatformat_for_type is explicitly checking for len==128. Is there some other float format matching this on i386? Perhaps checking the name is unnecessary. Also perhaps this could be fixed by adding some new gdbarch hook so that default_floatformat_for_type could handle this case. thanks, Tom
> >>>>> "Felix" == Willgerodt, Felix via Gdb-patches <gdb- > patches@sourceware.org> writes: > > Felix> || strcmp (name, "_Float128") == 0 > Felix> || strcmp (name, "complex _Float128") == 0 > Felix> || strcmp (name, "complex(kind=16)") == 0 > Felix> + || strcmp (name, "COMPLEX(16)") == 0 > Felix> || strcmp (name, "complex*32") == 0 > Felix> || strcmp (name, "COMPLEX*32") == 0 > Felix> || strcmp (name, "quad complex") == 0 > Felix> || strcmp (name, "real(kind=16)") == 0 > Felix> || strcmp (name, "real*16") == 0 > Felix> - || strcmp (name, "REAL*16") == 0) > Felix> + || strcmp (name, "REAL*16") == 0 > Felix> + || strcmp (name, "REAL(16)") == 0) > Felix> return floatformats_ieee_quad; > > >> Not your fault but this whole approach seems bad. > > Felix> Agreed. But can we change anything really? > > I suppose ideally what we would need is extra information in the DWARF. > > Although... i386_floatformat_for_type is explicitly checking for > len==128. Is there some other float format matching this on i386? As far as I know, the actual x87 FPU supports only an extended-double format with 80 bits and the SSE/AVX units only support double-precision. default_floatformat_for_type() has an explicit comment about it as well. When examining memory for this Fortran test, I see 128-bit values in binary128 format, so I assume that this is just a software/compiler feature. There is also decimal128 support in e.g. gcc. I tried it only briefly, we don't end up in i386_floatformat_for_type for those types, but GDB prints them just fine. I assume as they are marked as DW_ATE_decimal_float instead of DW_ATE_float, but I didn't look into it in detail. I am not aware of any other float types that exist with this length. Felix Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928
> When examining memory for this Fortran test, I see 128-bit values in > binary128 format, so I assume that this is just a software/compiler feature. > There is also decimal128 support in e.g. gcc. I tried it only briefly, > we don't end up in i386_floatformat_for_type for those types, but > GDB prints them just fine. I assume as they are marked as > DW_ATE_decimal_float instead of DW_ATE_float, but I didn't look into > it in detail. > I am not aware of any other float types that exist with this length. I wonder, then, if this patch would work? This way we would not have to curate a list of names. Tom diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index 476d17f1efd..57e50ffdd73 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8277,20 +8277,8 @@ static const struct floatformat ** i386_floatformat_for_type (struct gdbarch *gdbarch, const char *name, int len) { - if (len == 128 && name) - if (strcmp (name, "__float128") == 0 - || strcmp (name, "_Float128") == 0 - || strcmp (name, "complex _Float128") == 0 - || strcmp (name, "complex(kind=16)") == 0 - || strcmp (name, "COMPLEX(16)") == 0 - || strcmp (name, "complex*32") == 0 - || strcmp (name, "COMPLEX*32") == 0 - || strcmp (name, "quad complex") == 0 - || strcmp (name, "real(kind=16)") == 0 - || strcmp (name, "real*16") == 0 - || strcmp (name, "REAL*16") == 0 - || strcmp (name, "REAL(16)") == 0) - return floatformats_ieee_quad; + if (len == 128) + return floatformats_ieee_quad; return default_floatformat_for_type (gdbarch, name, len); }
> -----Original Message----- > From: Tom Tromey <tom@tromey.com> > Sent: Freitag, 24. Februar 2023 15:11 > To: Willgerodt, Felix via Gdb-patches <gdb-patches@sourceware.org> > Cc: Tom Tromey <tom@tromey.com>; Willgerodt, Felix > <felix.willgerodt@intel.com> > Subject: Re: [PATCH 1/1] gdb, fortran: Fix quad floating-point type for ifort > compiler. > > > When examining memory for this Fortran test, I see 128-bit values in > > binary128 format, so I assume that this is just a software/compiler feature. > > There is also decimal128 support in e.g. gcc. I tried it only briefly, > > we don't end up in i386_floatformat_for_type for those types, but > > GDB prints them just fine. I assume as they are marked as > > DW_ATE_decimal_float instead of DW_ATE_float, but I didn't look into > > it in detail. > > > I am not aware of any other float types that exist with this length. > > I wonder, then, if this patch would work? > This way we would not have to curate a list of names. > > Tom > > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index 476d17f1efd..57e50ffdd73 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -8277,20 +8277,8 @@ static const struct floatformat ** > i386_floatformat_for_type (struct gdbarch *gdbarch, > const char *name, int len) > { > - if (len == 128 && name) > - if (strcmp (name, "__float128") == 0 > - || strcmp (name, "_Float128") == 0 > - || strcmp (name, "complex _Float128") == 0 > - || strcmp (name, "complex(kind=16)") == 0 > - || strcmp (name, "COMPLEX(16)") == 0 > - || strcmp (name, "complex*32") == 0 > - || strcmp (name, "COMPLEX*32") == 0 > - || strcmp (name, "quad complex") == 0 > - || strcmp (name, "real(kind=16)") == 0 > - || strcmp (name, "real*16") == 0 > - || strcmp (name, "REAL*16") == 0 > - || strcmp (name, "REAL(16)") == 0) > - return floatformats_ieee_quad; > + if (len == 128) > + return floatformats_ieee_quad; > > return default_floatformat_for_type (gdbarch, name, len); > } I tested this quickly and see some regressions in gdb.base/complex-.parts.exp, gdb.base/bfp-test.exp and gdb.base/structs.exp (and a handful of others) on x86, with multiple compilers. Also gcc on Fedora 37 and Ubuntu 22.04. I didn't have the time to dig too deep. It seems like we return ieee_quad for some cases where we shouldn’t. (gdb) bt 5 #0 i386_floatformat_for_type (gdbarch=0x55555848f520, name=0x555558b5c760 "long double", len=128) at sources/gdb/gdb/i386-tdep.c:8281 #1 0x0000555555cd9e7e in gdbarch_floatformat_for_type (gdbarch=0x55555848f520, name=0x555558b5c760 "long double", length=128) at sources/gdb/gdb/gdbarch.c:1679 #2 0x0000555555f6fc9f in dwarf2_init_float_type (objfile=0x7fffe8021d00, bits=128, name=0x555558b5c760 "long double", name_hint=0x555558b5c760 "long double", byte_order=BFD_ENDIAN_LITTLE) at sources/gdb/gdb/dwarf2/read.c:15031 #3 0x0000555555f70543 in read_base_type (die=0x555558908f10, cu=0x7fffd8006970) at sources/gdb/gdb/dwarf2/read.c:15270 #4 0x0000555555f7b2be in read_type_die_1 (die=0x555558908f10, cu=0x7fffd8006970) at sources/gdb/gdb/dwarf2/read.c:19631 This seems to pass 128 bits for long double, where your patch did expect 80 bits. The 80 bit length for x86 is only handled in arch-utils.c:default_floatformat_for_type. I wonder if that could be streamlined a bit, not having to call gdbarch functions twice to figure out the format. I don't know why GDB passes 128 bit from the dwarf reader. I was debugging complex-parts only. In dwarf, we see the type having 256 bits. So maybe GDB splits it to 128 bit, as it is a complex? Felix Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index da4950ac259..acf22dba98d 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -8238,12 +8238,14 @@ i386_floatformat_for_type (struct gdbarch *gdbarch, || strcmp (name, "_Float128") == 0 || strcmp (name, "complex _Float128") == 0 || strcmp (name, "complex(kind=16)") == 0 + || strcmp (name, "COMPLEX(16)") == 0 || strcmp (name, "complex*32") == 0 || strcmp (name, "COMPLEX*32") == 0 || strcmp (name, "quad complex") == 0 || strcmp (name, "real(kind=16)") == 0 || strcmp (name, "real*16") == 0 - || strcmp (name, "REAL*16") == 0) + || strcmp (name, "REAL*16") == 0 + || strcmp (name, "REAL(16)") == 0) return floatformats_ieee_quad; return default_floatformat_for_type (gdbarch, name, len);