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.133.124]) by sourceware.org (Postfix) with ESMTPS id 429353858D39 for ; Tue, 12 Mar 2024 13:27:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 429353858D39 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 429353858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710250022; cv=none; b=RAZWGBclGRAA9yBpg/8IzM31ebIZRSzNZW7i/RMcC9PJFIeFK4utD4omNq4pfpjlHQ1h/AZ5WU0OWLKY/4cexsL+M03c34w5euSWLnx3Gn1EgTQJppMg+wqXgCJxv1ODW3eYVLauakZCKZ7T/9iBxyU8EDPMB4N+2j/C2phpAtc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710250022; c=relaxed/simple; bh=YBlgxZPmjJg7hJqoyYMtymTz4ZO3wcmY9zf0i8ZRK5Q=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=Nx/3mJYIs78u6h7MWpK9yG/nVVgju0cDmRWcF6q5c/NiJIy6nspB8QhvNM6EQqMRQIG5KfFcFOWZM6XiTScMT81AERzctLA7qHzsNU5aXR3HOD8dYj9ZGsXzs/GJMmNERr8hJYWeVc07MG0v3c79exAxYyE14P0UQqZWX864o2w= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710250019; 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=twH8gM7B9FzIYm+Vf3uedZ1X63komI27059OIHMJ3ag=; b=c4Fd+3e5hl0/bb26QSriF3mX3NrsB/o3wfkZWdUP26UNihBz8Vckvdo8Cg5fZzEQgO09o3 fXjS+wetVCb9W2RFkRpeZwPbLf0gjxGySaOURmURFumwg+jDIYfIM0Bs6+E2HhKKtWgEnG fzZAF9GHwNeELyf0Z2f/VJ7ild435vA= Received: from mail-oi1-f198.google.com (mail-oi1-f198.google.com [209.85.167.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-288-iF_pjOovMHKL68wXDODdUA-1; Tue, 12 Mar 2024 09:26:58 -0400 X-MC-Unique: iF_pjOovMHKL68wXDODdUA-1 Received: by mail-oi1-f198.google.com with SMTP id 5614622812f47-3c233e1b61fso5494308b6e.0 for ; Tue, 12 Mar 2024 06:26:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710250018; x=1710854818; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=twH8gM7B9FzIYm+Vf3uedZ1X63komI27059OIHMJ3ag=; b=PpKbUByTuDk244068ojfUfZQeNC5kmXJRCBXg65gm5h7fVaYZbGn3tMEaGXZLsA/1P oNCrEqZqbi7tril1AQvhi+hl55eoXLmgl/OH+txcxpTtvDn74coN+taffeXqenRPuGD2 aRz+zG18HkXLwiAa+6Wtdx3VkvpEwEobJQYSWA6Kl/v4Xaelmi87SQtVJSaoluODVAqk 6eLRv2X4zE4A4QhNhxeZxPIQGH5fMMVQeBq+JlADM1GRKGyE9qUyjPMRYOgbSLXYiome k0OReE/DThhBymkfeuE31zYJxom8V7hizN3wXfGoXOH+lapRidAaWc02vjruxo9OigAs HDhw== X-Gm-Message-State: AOJu0Yy/rcbDkeplAIDGGFCaHKGyVrLcIa36ce+hjRL5MsJBirh+kIyK uck0uILvJNARorbvHQCBDNlo4giqBUq1r8RDPFWXrLceu7Y0+197Jbke4Eyn8ml3dtRslDj1Yu5 P8KKytvX+dN5IK2crPAo9BRKkP0W9CkijvaIV3gSG5/lvXmNW75p/wj0= X-Received: by 2002:a05:6808:1155:b0:3c1:c7a8:5ede with SMTP id u21-20020a056808115500b003c1c7a85edemr13064640oiu.46.1710250017746; Tue, 12 Mar 2024 06:26:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEFW2oeltaXmsKJWW+oN9lEYrDJCyvBouQvmQ+zinuGKV8RCyVOQhYckV3SBWKVatg0Hc+cGw== X-Received: by 2002:a05:6808:1155:b0:3c1:c7a8:5ede with SMTP id u21-20020a056808115500b003c1c7a85edemr13064621oiu.46.1710250017428; Tue, 12 Mar 2024 06:26:57 -0700 (PDT) Received: from [192.168.1.130] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id x8-20020ac87308000000b0042f02284578sm3710351qto.68.2024.03.12.06.26.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 12 Mar 2024 06:26:56 -0700 (PDT) Message-ID: Date: Tue, 12 Mar 2024 09:26:56 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] To: Nathaniel Shead , Patrick Palka Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell References: <65ed9f0f.170a0220.64a2e.2144@mx.google.com> <65f04681.170a0220.35b00.1f01@mx.google.com> <65f048e4.170a0220.e7d67.23bd@mx.google.com> From: Jason Merrill In-Reply-To: <65f048e4.170a0220.e7d67.23bd@mx.google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.8 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,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 3/12/24 08:21, Nathaniel Shead wrote: > 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: OK. > 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); > +}