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 A71153858CD1 for ; Tue, 21 Nov 2023 15:57:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A71153858CD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A71153858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700582232; cv=none; b=FqlQUznIcU/JYmtUc+QFWuICOzvXt7aHVwLLdiJGy2GF13m3t19agi0qex4eNrU5YZKJpZ/J9JoJbleiwudNQecufI5MM+OgUCC4VQTSiKhuUntoIN0qxGQyjpZhPMgc0X6886oJLUaGztqG4WQrreSU45XWsbpHCyv+VygbVSo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700582232; c=relaxed/simple; bh=S5kL4bTSU7r4jwRaNoMqQaeKZwFiVjI8V6ZMbau27oA=; h=DKIM-Signature:Message-ID:Subject:From:To:Date:MIME-Version; b=vl3nfVrFgvtkR5skc89ZJ8K6wKRnblFfVKTxYkkgSARTVtKk8sbsVyuvbZ4SFlQGii+GeU/1Z4V6FEUjcnR7UKUIX6bVRCPRdQh9i2v3nZ1dB5oQFYt5YXAQCDPCDlMtrZdV6UeLWlYvGquKQE3KvxlBD6UAMhBPBkyw9C52FcQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700582230; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ywdzApSfwSY7CdCY0knBdB4mVrVzqTgkeFEEH6R2Wdw=; b=HnFY52sZKPYetwq/TWUxzWihwRQjrPaf3qw7sLz+R9+jvxQixorGs8Wxg3EsvW7AxKX8aa YqEuhKcyiQK8C5lgKnbI7pnXXZ74t46+S7/lHWg9a0S2zzSIzE9N1t/X/AkMmQkREDHuzg mb0Ce/acsvTaw6eqgRm8EZgZpb7EP1I= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-193-vEIdZ16TOrSYUcKDz_aD3g-1; Tue, 21 Nov 2023 10:56:56 -0500 X-MC-Unique: vEIdZ16TOrSYUcKDz_aD3g-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-41cdffe4d1cso68624961cf.0 for ; Tue, 21 Nov 2023 07:56:49 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700582208; x=1701187008; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=CvHohaB104fLCS489Z4wHWmd55XJefNndQ2ENUJsviw=; b=NcQS0DcK3FLQOUzjUspeRSgbFBSNC5mbcKxskgpxnzBlqJnFPfCgfQfyzqrcUzqRWw 9pMKQsX8xTAVj4VHRlJfjZYOSzDNbO1S6tCzRsV1qbD4COae6xIqBrg/vkcujhW0UM/I cSbd9dZRVn4lSQ3cluL310HWHTGs2rNTa3MQmMHk1qAoYLvbTLfhXqWrc8dgTKb2C59K /Hw/+0U7mSph9o04fLPLij5NDKKdVU2kCt1GTJfKImKCg7+n9z38HaFAcM4p+IIIsjY/ +Sn/SfLWTeLqsxH8r7KHlA1OoA5oi8VIH+aiz2FO9NgWOofGsky1GXZQHqCCDWk9czvB fAAQ== X-Gm-Message-State: AOJu0YyUdji1CaJ4MiWhVdGBNbV9VeKTsHDnm0I5V5ZUpvT9zISH15D4 Qqe51fEV33txqlddvHKwH2mKQWjOEspZ2QZ6tUs3i9JHo/WV6Ixz5c3On9QOG4EeEBhv/+ofFkb w/1mVMHU= X-Received: by 2002:ac8:68a:0:b0:423:76a8:4209 with SMTP id f10-20020ac8068a000000b0042376a84209mr727914qth.54.1700582208087; Tue, 21 Nov 2023 07:56:48 -0800 (PST) X-Google-Smtp-Source: AGHT+IGR8WXq5fpVGteFlx3UC6mV+bfG7rMgkIi0yodYYuEtV8Re0SS7k/AffDq63LIaHQ+6GzMX3g== X-Received: by 2002:ac8:68a:0:b0:423:76a8:4209 with SMTP id f10-20020ac8068a000000b0042376a84209mr727898qth.54.1700582207724; Tue, 21 Nov 2023 07:56:47 -0800 (PST) Received: from t14s.localdomain (c-76-28-97-5.hsd1.ma.comcast.net. [76.28.97.5]) by smtp.gmail.com with ESMTPSA id z13-20020ac875cd000000b0041b3a1462fbsm3688587qtq.37.2023.11.21.07.56.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Nov 2023 07:56:47 -0800 (PST) Message-ID: <6dee747a8195d1381fcd7c2455633cdaf53a1c32.camel@redhat.com> Subject: Re: [PATCH 2/2] jit: Complex types and loong constants From: David Malcolm To: Petter Tomner , jit@gcc.gnu.org Date: Tue, 21 Nov 2023 10:56:45 -0500 In-Reply-To: References: <697d6d25-c8fb-452b-8997-7ea28b6eb659@bahnhof.se> User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) MIME-Version: 1.0 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=-11.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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: On Mon, 2023-11-20 at 23:15 +0100, Petter Tomner wrote: > This patch adds the possibility to make the following constants: > * long long > * long double > * complex long double >=20 > The long long one is needed for 32bit systems to make 64bit > literals without union workarounds. >=20 > The new entrypoints are: > gcc_jit_context_new_rvalue_from_long_long > gcc_jit_context_new_rvalue_from_long_double > gcc_jit_context_new_rvalue_from_complex_long_double >=20 > The patch also fixes a issue with the reproducer's debug > c-file writer, which does not handle floating point numbers > very well. I.e. infs, NaN and losing precision on doubles. >=20 > make check-jit runs fine with the patch series on 64bit Debian > as well as on 32bit Debian in a VM, with a GNU/Linux setup. >=20 Thanks for this patch [...] > diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/= compatibility.rst > index 6922a2f67e9..e8b16926330 100644 > --- a/gcc/jit/docs/topics/compatibility.rst > +++ b/gcc/jit/docs/topics/compatibility.rst > @@ -384,13 +384,15 @@ alignment of a variable: > ``LIBGCCJIT_ABI_26`` > -------------------- > ``LIBGCCJIT_ABI_26`` covers the addition of support for complex types, > -literals for complex types as well as the > +literals for long long, long double and complex types as well as the > complex binary operator. You're extending the definition of the numbered ABI here. I'm OK with that if the 2nd patch goes in immediately after the 1st. [...] > template <> > diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc > index ed788502c15..baa75311b31 100644 > --- a/gcc/jit/jit-recording.cc > +++ b/gcc/jit/jit-recording.cc > @@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. If not see > #include "toplev.h" > =20 > #include > +#include > =20 > #include "jit-builtins.h" > #include "jit-recording.h" > @@ -1836,8 +1837,48 @@ recording::context::dump_reproducer_to_file (const= char *path) > =09 " gcc_jit_context_dump_reproducer_to_file.\n\n"); > print_version (r.get_file (), " ", false); > r.write ("*/\n"); > - r.write ("#include \n\n"); > - r.write ("#pragma GCC diagnostic ignored \"-Wunused-variable\"\n\n"); > + r.write ( > + "#include \n" > + "#include \n" > + "#include \n" > + "#include \n\n"); > + r.write ("#pragma GCC diagnostic ignored \"-Wunused-variable\"\n"); > + r.write ("#pragma GCC diagnostic ignored \"-Wunused-function\"\n\n"); > + /* Type punning unions */ > + r.write ("union dull { double d; unsigned long long ull; };\n\n"); > + r.write ( > + "union ldarr { long double ld; unsigned arr[sizeof (long double)];};= \n\n"); > + > + /* Functions for type punning nan:s, keeping bit representation */ > + r.write ( > + "/* Convert type punned uint 64 to double NaN.\n" > + " Might lose/corrupt payload between architectures. */\n" > + "static double\n" > + "ull_to_d (unsigned long long ull)\n" > + "{\n" > + " union dull u; u.ull =3D ull;\n" > + " /* Paranoia check for foreign NaN representation. */\n" > + " if (!isnan (u.d)) return NAN;\n" > + " return u.d;\n" > + "}\n\n"); > + r.write ( > + "/* Convert array to long double NaN.\n" > + " Might lose/corrupt payload between architectures. */\n" > + "static long double\n" > + "arr_to_ld (unsigned char *arr, int len)\n" > + "{\n" > + " union ldarr u;\n" > + /* Since long double sizes can vary between architectures, > + we need to handle that. */ > + " /* For foreign long double sizes */\n" > + " if (sizeof u.arr !=3D len) return NAN;\n" > + " memcpy (u.arr, arr, len);\n" > + /* The size might fool us becouse of padding, so check it is a nan. = */ > + " /* Handle foreign representations that is not NaN on this machine= . */\n" > + " if (!isnan (u.ld)) return NAN;\n" > + " return u.ld;\n" > + "}\n\n"); There are various places above that make the reproducer rquire C99/C++11: - is C99 onwards - "long long" is C99 onwards - "isnan" and "NAN" are C99 and C++11. Perhaps we could add preprocessor guards here. But maybe the things that need these are only expressable in code that was written in that version of the language, so maybe only write out these things if they're needed? That might eliminate a lot of preprocessor gunk in the generated reproducer, at the cost of some complexity in the dump_reproducer logic. Or we could simply require the user to use C99/C++11 for generated reproducers. That's probably simplest. [...] > +/* For typepruning NaNs. */ ^^^^^^^^^^^ Is this a typo for "typepunning"... > +union dull { > + double d; > + unsigned long long ull; > +}; > + > +union ldarr { > + long double ld; > + unsigned char arr[sizeof(long double)]; > +}; > + > + ...here? [...] > +/* Helper function for write_reproducer() > + > + Write an array of unsigned chars to the reproducer, like: > + {0x12, 0x34, ... } */ > +static void > +reproducer_write_arr (reproducer &r, unsigned char *arr, int len) "arr" should probably be const, and "len" be a size_t. > +{ > + if (len =3D=3D 0) > + return; > + > + r.write ("{"); > + for (int i =3D 0; i < len - 1; i++) > + r.write ("%#x , ", arr[i]); > + r.write ("%#x }", arr[len - 1]); > +} [...] > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h > index cdaec450981..2134c92465e 100644 > --- a/gcc/jit/libgccjit.h > +++ b/gcc/jit/libgccjit.h > @@ -1069,6 +1069,16 @@ gcc_jit_context_new_rvalue_from_long (gcc_jit_cont= ext *ctxt, > =09=09=09=09 gcc_jit_type *numeric_type, > =09=09=09=09 long value); > =20 > +#define LIBGCCJIT_HAVE_LONGLONG_CONSTANTS > +/* Get a constant from a long long. > + > + This function was added in LIBGCCJIT_ABI_26. You can test for > + its presence with: > + #ifdef LIBGCCJIT_HAVE_LONGLONG_CONSTANTS */ > +extern gcc_jit_rvalue * > +gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt, > +=09=09=09=09=09 gcc_jit_type *numeric_type, > +=09=09=09=09=09 long long value); "long long" is C99 / C++11; can we add a suitable preprocessor guard for this declaration? [...] > +/* Get a constant from a long double. > + > + This function was added in LIBGCCJIT_ABI_26. You can test for > + its presence with: > + #ifdef LIBGCCJIT_HAVE_LONGLONG_CONSTANTS */ > +extern gcc_jit_rvalue * > +gcc_jit_context_new_rvalue_from_long_double (gcc_jit_context *ctxt, > +=09=09=09=09=09 gcc_jit_type *numeric_type, > +=09=09=09=09=09 long double value); FWIW "LIBGCCJIT_HAVE_LONGLONG_CONSTANTS" doesn't seem quite like the right name here, but I wasn't able to come up with a better one. > +/* Get a constant from a complex double. > =20 > This API entrypoint was added in LIBGCCJIT_ABI_26; you can test for i= ts > presence using > @@ -1093,6 +1113,17 @@ gcc_jit_context_new_rvalue_from_complex_double (gc= c_jit_context *ctxt, > =09=09=09=09=09=09gcc_jit_type *numeric_type, > =09=09=09=09=09=09_Complex double value); > =20 > +/* Get a constant from a complex long double. > + > + This API entrypoint was added in LIBGCCJIT_ABI_26; you can test for i= ts > + presence using > + #ifdef LIBGCCJIT_HAVE_LONGLONG_CONSTANTS */ Likewise here. > +extern gcc_jit_rvalue * > +gcc_jit_context_new_rvalue_from_complex_long_double ( > + gcc_jit_context *ctxt, > + gcc_jit_type *numeric_type, > + _Complex long double value); > + As noted in the review of the other patch "_Complex", is C99 (and I don't know which C++ version), so some kind of preprocessor guard would be good here, so that we don't impose a requirement on all users of libgccjit.h [...] As before, I looked through the rest of the patch and it seems OK. Thanks again for the patch Dave