* [PATCH] c++/modules: Support target-specific nodes with streaming [PR111224] @ 2024-03-10 11:52 Nathaniel Shead 2024-03-11 14:36 ` Patrick Palka 0 siblings, 1 reply; 6+ messages in thread From: Nathaniel Shead @ 2024-03-10 11:52 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell, Patrick Palka 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. This patch doesn't provide "full" support of AArch64 SVE, however, since for that we would need to support 'target' nodes (tracked in PR108080). 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 target-specific builtin types, and vectors with NUM_POLY_INT_COEFFS > 1. (trees_in::tree_node): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/pr111224_a.C: New test. * g++.dg/modules/pr111224_b.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/module.cc | 70 +++++++++++++++++++---- gcc/testsuite/g++.dg/modules/pr111224_a.C | 17 ++++++ gcc/testsuite/g++.dg/modules/pr111224_b.C | 13 +++++ 3 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/pr111224_a.C create mode 100644 gcc/testsuite/g++.dg/modules/pr111224_b.C diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 99055523d91..0b5e2e67053 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -2718,6 +2718,7 @@ enum tree_tag { tt_typedef_type, /* A (possibly implicit) typedefed type. */ tt_derived_type, /* A type derived from another type. */ tt_variant_type, /* A variant of another type. */ + tt_builtin_type, /* A custom builtin type. */ tt_tinfo_var, /* Typeinfo object. */ tt_tinfo_typedef, /* Typeinfo typedef. */ @@ -2732,7 +2733,7 @@ enum tree_tag { tt_binfo, /* A BINFO. */ tt_vtable, /* A vtable. */ tt_thunk, /* A thunk. */ - tt_clone_ref, + tt_clone_ref, /* A cloned function. */ tt_entity, /* A extra-cluster entity. */ @@ -5173,7 +5174,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 +5259,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 +6105,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 +6617,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))) @@ -8930,6 +8933,32 @@ trees_out::type_node (tree type) return; } + if (tree name = TYPE_NAME (type)) + if (TREE_CODE (name) == TYPE_DECL && DECL_ARTIFICIAL (name)) + { + /* Potentially a custom machine- or OS-specific builtin type. */ + bool found = false; + unsigned ix = 0; + for (tree t = registered_builtin_types; t; t = TREE_CHAIN (t), ix++) + if (TREE_VALUE (t) == type) + { + found = true; + break; + } + if (found) + { + int type_tag = insert (type); + if (streaming_p ()) + { + i (tt_builtin_type); + u (ix); + dump (dumper::TREE) + && dump ("Wrote:%d builtin type %N", type_tag, name); + } + return; + } + } + if (streaming_p ()) { u (tt_derived_type); @@ -9068,8 +9097,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<unsigned HOST_WIDE_INT> (nunits.to_constant ())); + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) + wu (nunits.coeffs[ix]); } break; } @@ -9630,9 +9659,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<poly_int64> (nunits)); + res = build_vector_type (res, nunits); } break; } @@ -9700,6 +9731,25 @@ trees_in::tree_node (bool is_use) } break; + case tt_builtin_type: + /* A machine- or OS-specific builtin type. */ + { + unsigned ix = u (); + res = registered_builtin_types; + for (; ix && res; res = TREE_CHAIN (res)) + ix--; + if (!res) + set_overrun (); + if (!get_overrun ()) + { + res = TREE_VALUE (res); + int type_tag = insert (res); + dump (dumper::TREE) + && dump ("Read:%d builtin type %N", type_tag, res); + } + } + break; + case tt_tinfo_var: case tt_tinfo_typedef: /* A tinfo var or typedef. */ diff --git a/gcc/testsuite/g++.dg/modules/pr111224_a.C b/gcc/testsuite/g++.dg/modules/pr111224_a.C new file mode 100644 index 00000000000..6c699053cdc --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr111224_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 <arm_sve.h> + +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/pr111224_b.C b/gcc/testsuite/g++.dg/modules/pr111224_b.C new file mode 100644 index 00000000000..c18691dcf8a --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/pr111224_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 <arm_sve.h> +import M; + +int main() { + svbool_t x = svptrue_b8 (); + svfloat16_t f = svdup_n_f16(1.0); + foo(x, f); +} -- 2.43.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/modules: Support target-specific nodes with streaming [PR111224] 2024-03-10 11:52 [PATCH] c++/modules: Support target-specific nodes with streaming [PR111224] Nathaniel Shead @ 2024-03-11 14:36 ` Patrick Palka 2024-03-12 12:11 ` [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] Nathaniel Shead 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2024-03-11 14:36 UTC (permalink / raw) To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell, Patrick Palka 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? > > This patch doesn't provide "full" support of AArch64 SVE, however, since > for that we would need to support 'target' nodes (tracked in PR108080). > > 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 target-specific builtin types, > and vectors with NUM_POLY_INT_COEFFS > 1. > (trees_in::tree_node): Likewise. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/pr111224_a.C: New test. > * g++.dg/modules/pr111224_b.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/module.cc | 70 +++++++++++++++++++---- > gcc/testsuite/g++.dg/modules/pr111224_a.C | 17 ++++++ > gcc/testsuite/g++.dg/modules/pr111224_b.C | 13 +++++ > 3 files changed, 90 insertions(+), 10 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/pr111224_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/pr111224_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 99055523d91..0b5e2e67053 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -2718,6 +2718,7 @@ enum tree_tag { > tt_typedef_type, /* A (possibly implicit) typedefed type. */ > tt_derived_type, /* A type derived from another type. */ > tt_variant_type, /* A variant of another type. */ > + tt_builtin_type, /* A custom builtin type. */ > > tt_tinfo_var, /* Typeinfo object. */ > tt_tinfo_typedef, /* Typeinfo typedef. */ > @@ -2732,7 +2733,7 @@ enum tree_tag { > tt_binfo, /* A BINFO. */ > tt_vtable, /* A vtable. */ > tt_thunk, /* A thunk. */ > - tt_clone_ref, > + tt_clone_ref, /* A cloned function. */ > > tt_entity, /* A extra-cluster entity. */ > > @@ -5173,7 +5174,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 +5259,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 +6105,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 +6617,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))) > @@ -8930,6 +8933,32 @@ trees_out::type_node (tree type) > return; > } > > + if (tree name = TYPE_NAME (type)) > + if (TREE_CODE (name) == TYPE_DECL && DECL_ARTIFICIAL (name)) > + { > + /* Potentially a custom machine- or OS-specific builtin type. */ > + bool found = false; > + unsigned ix = 0; > + for (tree t = registered_builtin_types; t; t = TREE_CHAIN (t), ix++) > + if (TREE_VALUE (t) == type) > + { > + found = true; > + break; > + } > + if (found) > + { > + int type_tag = insert (type); > + if (streaming_p ()) > + { > + i (tt_builtin_type); > + u (ix); > + dump (dumper::TREE) > + && dump ("Wrote:%d builtin type %N", type_tag, name); > + } > + return; > + } > + } > + > if (streaming_p ()) > { > u (tt_derived_type); > @@ -9068,8 +9097,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<unsigned HOST_WIDE_INT> (nunits.to_constant ())); > + for (unsigned ix = 0; ix != NUM_POLY_INT_COEFFS; ix++) > + wu (nunits.coeffs[ix]); > } > break; > } > @@ -9630,9 +9659,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<poly_int64> (nunits)); > + res = build_vector_type (res, nunits); > } > break; > } > @@ -9700,6 +9731,25 @@ trees_in::tree_node (bool is_use) > } > break; > > + case tt_builtin_type: > + /* A machine- or OS-specific builtin type. */ > + { > + unsigned ix = u (); > + res = registered_builtin_types; > + for (; ix && res; res = TREE_CHAIN (res)) > + ix--; > + if (!res) > + set_overrun (); > + if (!get_overrun ()) > + { > + res = TREE_VALUE (res); > + int type_tag = insert (res); > + dump (dumper::TREE) > + && dump ("Read:%d builtin type %N", type_tag, res); > + } > + } > + break; > + > case tt_tinfo_var: > case tt_tinfo_typedef: > /* A tinfo var or typedef. */ > diff --git a/gcc/testsuite/g++.dg/modules/pr111224_a.C b/gcc/testsuite/g++.dg/modules/pr111224_a.C > new file mode 100644 > index 00000000000..6c699053cdc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr111224_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 <arm_sve.h> > + > +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/pr111224_b.C b/gcc/testsuite/g++.dg/modules/pr111224_b.C > new file mode 100644 > index 00000000000..c18691dcf8a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr111224_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 <arm_sve.h> > +import M; > + > +int main() { > + svbool_t x = svptrue_b8 (); > + svfloat16_t f = svdup_n_f16(1.0); > + foo(x, f); > +} > -- > 2.43.2 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] 2024-03-11 14:36 ` Patrick Palka @ 2024-03-12 12:11 ` Nathaniel Shead 2024-03-12 12:21 ` Nathaniel Shead 0 siblings, 1 reply; 6+ messages in thread From: Nathaniel Shead @ 2024-03-12 12:11 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell 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 <nathanieloshead@gmail.com> Reviewed-by: Patrick Palka <ppalka@redhat.com> --- 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<unsigned HOST_WIDE_INT> (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<poly_int64> (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 <arm_sve.h> + +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 <arm_sve.h> +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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] 2024-03-12 12:11 ` [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] Nathaniel Shead @ 2024-03-12 12:21 ` Nathaniel Shead 2024-03-12 12:52 ` Patrick Palka 2024-03-12 13:26 ` Jason Merrill 0 siblings, 2 replies; 6+ messages in thread From: Nathaniel Shead @ 2024-03-12 12:21 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell 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 <nathanieloshead@gmail.com> > Reviewed-by: Patrick Palka <ppalka@redhat.com> > --- > 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<unsigned HOST_WIDE_INT> (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<poly_int64> (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 <arm_sve.h> > + > +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 <arm_sve.h> > +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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] 2024-03-12 12:21 ` Nathaniel Shead @ 2024-03-12 12:52 ` Patrick Palka 2024-03-12 13:26 ` Jason Merrill 1 sibling, 0 replies; 6+ messages in thread From: Patrick Palka @ 2024-03-12 12:52 UTC (permalink / raw) To: Nathaniel Shead; +Cc: Patrick Palka, gcc-patches, Jason Merrill, Nathan Sidwell On Tue, 12 Mar 2024, 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 <nathanieloshead@gmail.com> > > Reviewed-by: Patrick Palka <ppalka@redhat.com> > > --- > > 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<unsigned HOST_WIDE_INT> (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<poly_int64> (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 <arm_sve.h> > > + > > +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 <arm_sve.h> > > +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: Sweet! LGTM > > 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 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] 2024-03-12 12:21 ` Nathaniel Shead 2024-03-12 12:52 ` Patrick Palka @ 2024-03-12 13:26 ` Jason Merrill 1 sibling, 0 replies; 6+ messages in thread From: Jason Merrill @ 2024-03-12 13:26 UTC (permalink / raw) To: Nathaniel Shead, Patrick Palka; +Cc: gcc-patches, Nathan Sidwell 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 <nathanieloshead@gmail.com> >> Reviewed-by: Patrick Palka <ppalka@redhat.com> >> --- >> 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<unsigned HOST_WIDE_INT> (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<poly_int64> (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 <arm_sve.h> >> + >> +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 <arm_sve.h> >> +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); > +} ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-12 13:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-10 11:52 [PATCH] c++/modules: Support target-specific nodes with streaming [PR111224] Nathaniel Shead 2024-03-11 14:36 ` Patrick Palka 2024-03-12 12:11 ` [PATCH v2] c++: Support target-specific nodes with streaming [PR98645,PR111224] Nathaniel Shead 2024-03-12 12:21 ` Nathaniel Shead 2024-03-12 12:52 ` Patrick Palka 2024-03-12 13:26 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).