| Message ID | 65ed9f0f.170a0220.64a2e.2144@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: Support target-specific nodes with streaming [PR111224] | expand |
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 > >
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); +}
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