From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id 5DCB93858281 for ; Tue, 12 Mar 2024 12:21:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5DCB93858281 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5DCB93858281 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::629 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710246122; cv=none; b=KXpTHOjv330HyaLfDExt6wF5IfIFtYvwBRJrzk31B34y3Xsf8hHlA56AvEshb4gA+TqYPEPfFXw2wzGKn8id1iBc3DIRtm66MPbqqWylPHb70t1qGQhKY1dYaR+MmMAuQslANOvaLZfIv+N8Ju9OK8wBbfkYOqOZ+LtyjOkbVEU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710246122; c=relaxed/simple; bh=El1wWYTvJKxYRpUs9gzndYJJPey//Xs581hAClMhdE0=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=eERPtOnREOoGQYF0u7dM9VeAbbpqy/7L72PLOPARwg83GYQkIFNCjqcjnVTItalsuZOAFyalevQEjFVlLinAjOQ1G8Tk3N3ARWFfWPc9q/mAj3vhIJsXMhifZfyP+Fhce/V/qJbzqHHN5oCOfRcV6E4I78vDYo8Gr0FujKpAcp0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x629.google.com with SMTP id d9443c01a7336-1dd6198c4e2so38408205ad.2 for ; Tue, 12 Mar 2024 05:21:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710246117; x=1710850917; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=rJ0G76cGV0uUHukNUO0qxTvDHsJuNPL6k4zjsaP9Wpw=; b=B55C8aHkugb38ZsXNKBkj5carTE6OvrnN6hBlcW0bFQSluGtQxOwMICNP8+ApxC90O Ctf9ZbtwtfpnkFKdau8K15F4rj0XJYgAkekfnEtaWlBBSnj4iOiHwc71DyVZUcmd7Pnd HIrsxfE72PDWOhu3x4xBTpQU8q1eXdcGpHZJbyqI06uhEZ6MVMppE36N/2vTpDf6fZsT WwnjrPEn8MRMr0y3T52W6+sdnxgaRgMKgJedu4WU//xL6fw9Ikh+j6a2TmGMiDbDA6pU mqxmhnJg65e6CsTqyIC75SOCay+pF0f1fpMn6tjf74RfEslcMNlIZq+kNiqsTrwACrrB nR+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710246117; x=1710850917; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=rJ0G76cGV0uUHukNUO0qxTvDHsJuNPL6k4zjsaP9Wpw=; b=b56uETz+mG7xc01cHd5OYRbOCDtUAi14U7dD9oClZC76usT1YN5//A5pUityohSneA yQt8LNfmMcOy4fPM87dV2NhO3B8YJPm+j4U+qkfaEtN86e1RgN1oY7zENXxBwZi319bH Mqu8pewjuf+lYw7F1aSvIV7Rt/HXQuLOnCtfioOHJg0405eTAKyrRZWeWl01MiVtbpFZ T9D7sXIkC2SssNwvCGU8PYwBCM4y8F5D/7goqFzMo4EqKPSg+8JGOamzDo4Gr4Q0mQaT WORh56uQyigqfBtGRrDihjQpzKaOF8Cc03J8M5rEzQcs94lTAm+WwE1vSH/5GCZMt08i 3LnQ== X-Gm-Message-State: AOJu0YydlU3CdcNdyHjqTl//pWNVw83fo+Z2l7cOBc0hsC5JxNxsRs2f PknVu1wjEgg8TbUfSexXXMOT8Y1ym3x3zwbmNgY9lou/W5vFbbbp X-Google-Smtp-Source: AGHT+IHJRYU/xX6LUlQgUv1ZbIFyDGyobk1Yqy8p1SzFxQEb7xWWJlqgPTHmhopZTcdAsGYO2eSVlA== X-Received: by 2002:a17:902:748a:b0:1dd:7163:b58e with SMTP id h10-20020a170902748a00b001dd7163b58emr8960822pll.41.1710246117160; Tue, 12 Mar 2024 05:21:57 -0700 (PDT) Received: from Thaum. (202-161-100-107.tpgi.com.au. [202.161.100.107]) by smtp.gmail.com with ESMTPSA id b10-20020a170902650a00b001dd6f1516a0sm6747831plk.87.2024.03.12.05.21.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Mar 2024 05:21:56 -0700 (PDT) Message-ID: <65f048e4.170a0220.e7d67.23bd@mx.google.com> X-Google-Original-Message-ID: Date: Tue, 12 Mar 2024 23:21:51 +1100 From: Nathaniel Shead To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, Jason Merrill , Nathan Sidwell Subject: Re: [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] References: <65ed9f0f.170a0220.64a2e.2144@mx.google.com> <65f04681.170a0220.35b00.1f01@mx.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <65f04681.170a0220.35b00.1f01@mx.google.com> X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,URIBL_BLACK 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 Tue, Mar 12, 2024 at 11:11:40PM +1100, Nathaniel Shead wrote: > On Mon, Mar 11, 2024 at 10:36:06AM -0400, Patrick Palka wrote: > > On Sun, 10 Mar 2024, Nathaniel Shead wrote: > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu and > > > aarch64-unknown-linux-gnu, OK for trunk? > > > > > > It's worth noting that the AArch64 machines I had available to test with > > > didn't have a new enough glibc to reproduce the ICEs in the PR, but this > > > patch will be necessary (albeit possibly not sufficient) to fix it. > > > > > > -- >8 -- > > > > > > Some targets make use of POLY_INT_CSTs and other custom builtin types, > > > which currently violate some assumptions when streaming. This patch adds > > > support for them, specifically AArch64 SVE types like __fp16. > > > > It seems other built-in types are handled by adding them to the > > fixed_trees vector in init_modules (and then we install them first > > during streaming). Could we just add all the target-specific types to > > fixed_trees too? > > > > Yes, that works too. Seems cleaner as well, though I had to add it as a > separate loop because the set of builtin types registered is not > determined until runtiem (depending on e.g. ABI flags). I also noticed > that this fixes another PR, on PowerPC, so I've added a test for it. > Thanks! > > Bootstrapped and regtested on x86_64-pc-linux-gnu, > aarch64-unknown-linux-gnu, and powerpc64le-unknown-linux-gnu; > OK for trunk? > > -- >8 -- > > Some targets make use of POLY_INT_CSTs and other custom builtin types, > which currently violate some assumptions when streaming. This patch adds > support for them, such as types like Aarch64 __fp16, PowerPC __ibm128, > and vector types thereof. > > This patch doesn't provide "full" support of AArch64 SVE, however, since > for that we would need to support 'target' nodes (tracked in PR108080). > > Adding the new builtin types means that on Aarch64 we now have 217 > global trees created on initialisation (up from 191), so this patch also > slightly bumps the initial size of the fixed_trees allocation to 250. > > PR c++/98645 > PR c++/111224 > > gcc/cp/ChangeLog: > > * module.cc (enum tree_tag): Add new tag for builtin types. > (trees_out::start): POLY_INT_CSTs can be emitted. > (trees_in::start): Likewise. > (trees_out::core_vals): Stream POLY_INT_CSTs. > (trees_in::core_vals): Likewise. > (trees_out::type_node): Handle vectors with multiple coeffs. > (trees_in::tree_node): Likewise. > (init_modules): Register target-specific builtin types. Bump > initial capacity slightly. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/target-aarch64-1_a.C: New test. > * g++.dg/modules/target-aarch64-1_b.C: New test. > * g++.dg/modules/target-powerpc-1_a.C: New test. > * g++.dg/modules/target-powerpc-1_b.C: New test. > > Signed-off-by: Nathaniel Shead > Reviewed-by: Patrick Palka > --- > gcc/cp/module.cc | 32 +++++++++++++------ > .../g++.dg/modules/target-aarch64-1_a.C | 17 ++++++++++ > .../g++.dg/modules/target-aarch64-1_b.C | 13 ++++++++ > .../g++.dg/modules/target-powerpc-1_a.C | 7 ++++ > .../g++.dg/modules/target-powerpc-1_b.C | 10 ++++++ > 5 files changed, 69 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/target-aarch64-1_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/target-aarch64-1_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/target-powerpc-1_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/target-powerpc-1_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 99055523d91..8aab9ea0bae 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5173,7 +5173,6 @@ trees_out::start (tree t, bool code_streamed) > break; > > case FIXED_CST: > - case POLY_INT_CST: > gcc_unreachable (); /* Not supported in C++. */ > break; > > @@ -5259,7 +5258,6 @@ trees_in::start (unsigned code) > > case FIXED_CST: > case IDENTIFIER_NODE: > - case POLY_INT_CST: > case SSA_NAME: > case TARGET_MEM_REF: > case TRANSLATION_UNIT_DECL: > @@ -6106,7 +6104,10 @@ trees_out::core_vals (tree t) > break; > > case POLY_INT_CST: > - gcc_unreachable (); /* Not supported in C++. */ > + if (streaming_p ()) > + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) > + WT (POLY_INT_CST_COEFF (t, ix)); > + break; > > case REAL_CST: > if (streaming_p ()) > @@ -6615,8 +6616,9 @@ trees_in::core_vals (tree t) > break; > > case POLY_INT_CST: > - /* Not suported in C++. */ > - return false; > + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) > + RT (POLY_INT_CST_COEFF (t, ix)); > + break; > > case REAL_CST: > if (const void *bytes = buf (sizeof (real_value))) > @@ -9068,8 +9070,8 @@ trees_out::type_node (tree type) > if (streaming_p ()) > { > poly_uint64 nunits = TYPE_VECTOR_SUBPARTS (type); > - /* to_constant asserts that only coeff[0] is of interest. */ > - wu (static_cast (nunits.to_constant ())); > + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) > + wu (nunits.coeffs[ix]); > } > break; > } > @@ -9630,9 +9632,11 @@ trees_in::tree_node (bool is_use) > > case VECTOR_TYPE: > { > - unsigned HOST_WIDE_INT nunits = wu (); > + poly_uint64 nunits; > + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) > + nunits.coeffs[ix] = wu (); > if (!get_overrun ()) > - res = build_vector_type (res, static_cast (nunits)); > + res = build_vector_type (res, nunits); > } > break; > } > @@ -20151,7 +20155,7 @@ init_modules (cpp_reader *reader) > some global trees are lazily created and we don't want that to > mess with our syndrome of fixed trees. */ > unsigned crc = 0; > - vec_alloc (fixed_trees, 200); > + vec_alloc (fixed_trees, 250); > > dump () && dump ("+Creating globals"); > /* Insert the TRANSLATION_UNIT_DECL. */ > @@ -20169,6 +20173,14 @@ init_modules (cpp_reader *reader) > dump () && dump ("+%u", v); > } > } > + /* OS- and machine-specific types are dynamically registered at > + runtime, so cannot be part of global_tree_arys. */ > + registered_builtin_types && dump ("") && dump ("+\tB:"); > + for (tree t = registered_builtin_types; t; t = TREE_CHAIN (t)) > + { > + unsigned v = maybe_add_global (TREE_VALUE (t), crc); > + dump () && dump ("+%u", v); > + } > global_crc = crc32_unsigned (crc, fixed_trees->length ()); > dump ("") && dump ("Created %u unique globals, crc=%x", > fixed_trees->length (), global_crc); > diff --git a/gcc/testsuite/g++.dg/modules/target-aarch64-1_a.C b/gcc/testsuite/g++.dg/modules/target-aarch64-1_a.C > new file mode 100644 > index 00000000000..6c699053cdc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/target-aarch64-1_a.C > @@ -0,0 +1,17 @@ > +// PR c++/111224 > +// { dg-do compile { target aarch64*-*-* } } > +// { dg-require-effective-target aarch64_asm_sve_ok } > +// { dg-additional-options "-fmodules-ts -march=armv8.2-a+sve" } > + > +module; > + > +// We can't do a header unit of this right now because this > +// uses target attributes, that we don't yet support. > +// See also PR c++/108080. > +#include > + > +export module M; > + > +export inline void foo(svbool_t x, svfloat16_t f) { > + svabs_f16_x(x, f); > +} > diff --git a/gcc/testsuite/g++.dg/modules/target-aarch64-1_b.C b/gcc/testsuite/g++.dg/modules/target-aarch64-1_b.C > new file mode 100644 > index 00000000000..c18691dcf8a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/target-aarch64-1_b.C > @@ -0,0 +1,13 @@ > +// PR c++/111224 > +// { dg-module-do link { target aarch64*-*-* } } > +// { dg-require-effective-target aarch64_asm_sve_ok } > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -march=armv8.2-a+sve" } > + > +#include > +import M; > + > +int main() { > + svbool_t x = svptrue_b8 (); > + svfloat16_t f = svdup_n_f16(1.0); > + foo(x, f); > +} > diff --git a/gcc/testsuite/g++.dg/modules/target-powerpc-1_a.C b/gcc/testsuite/g++.dg/modules/target-powerpc-1_a.C > new file mode 100644 > index 00000000000..693ed101ed5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/target-powerpc-1_a.C > @@ -0,0 +1,7 @@ > +// PR c++/98645 > +// { dg-do compile { target powerpc*-*-* } } > +// { dg-require-effective-target ppc_float128_sw } > +// { dg-additional-options "-fmodules-ts -mfloat128 -mabi=ieeelongdouble" } > + > +export module M; > +export __ibm128 i = 0.0; > diff --git a/gcc/testsuite/g++.dg/modules/target-powerpc-1_b.C b/gcc/testsuite/g++.dg/modules/target-powerpc-1_b.C > new file mode 100644 > index 00000000000..d6b684b556d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/target-powerpc-1_b.C > @@ -0,0 +1,10 @@ > +// PR c++/98645 > +// { dg-module-do compile { target powerpc*-*-* } } > +// { dg-require-effective-target ppc_float128_sw } > +// { dg-additional-options "-fmodules-ts -mfloat128 -mabi=ieeelongdouble" } > + > +import M; > + > +int main() { > + __ibm128 j = i; > +} > -- > 2.43.2 > Actually just noticed another PR this also seems to fix, PR c++/98688; here are another two testcases I'll include in the above patch: diff --git a/gcc/testsuite/g++.dg/modules/target-powerpc-2_a.C b/gcc/testsuite/g++.dg/modules/target-powerpc-2_a.C new file mode 100644 index 00000000000..cc18862e55c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/target-powerpc-2_a.C @@ -0,0 +1,20 @@ +// PR c++/98688 +// { dg-do compile { target powerpc*-*-* } } +// { dg-additional-options "-fmodules-ts -mcpu=power10 -mmma" } + +export module mma_foo0; + +typedef unsigned char vec_t __attribute__((vector_size(16))); + +export void +foo0 (__vector_quad *dst, vec_t *vec, __vector_pair *pvecp) +{ + __vector_quad acc; + __vector_pair vecp0 = *pvecp; + vec_t vec1 = vec[1]; + + __builtin_mma_xvf64ger (&acc, vecp0, vec1); + __builtin_mma_xvf64gerpp (&acc, vecp0, vec1); + __builtin_mma_xvf64gerpn (&acc, vecp0, vec1); + dst[0] = acc; +} diff --git a/gcc/testsuite/g++.dg/modules/target-powerpc-2_b.C b/gcc/testsuite/g++.dg/modules/target-powerpc-2_b.C new file mode 100644 index 00000000000..9e77ba7afca --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/target-powerpc-2_b.C @@ -0,0 +1,12 @@ +// PR c++/98688 +// { dg-module-do compile { target powerpc*-*-* } } +// { dg-additional-options "-fmodules-ts -mcpu=power10 -mmma" } + +import mma_foo0; + +typedef unsigned char vec_t __attribute__((vector_size(16))); + +void bar(__vector_quad *dst, vec_t *vec, __vector_pair *pvecp) +{ + foo0 (dst, vec, pvecp); +} -- 2.43.2