middle-end/102360 - adjust .DEFERRED_INIT expansion
Commit Message
This avoids using native_interpret_type when we cannot do it with
the original type of the variable, instead use an integer type
for the initialization and side-step the size limitation of
native_interpret_int.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress
(note the reported ICE happens on aarch64 only)
Richard.
2021-09-16 Richard Biener <rguenther@suse.de>
PR middle-end/102360
* internal-fn.c (expand_DEFERRED_INIT): Make pattern-init
of non-memory more robust.
* g++.dg/pr102360.C: New testcase.
---
gcc/internal-fn.c | 24 ++++++---------
gcc/testsuite/g++.dg/pr102360.C | 54 +++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr102360.C
Comments
On Thu, Sep 16, 2021 at 11:48:49AM +0200, Richard Biener via Gcc-patches wrote:
> 2021-09-16 Richard Biener <rguenther@suse.de>
>
> PR middle-end/102360
> * internal-fn.c (expand_DEFERRED_INIT): Make pattern-init
> of non-memory more robust.
>
> * g++.dg/pr102360.C: New testcase.
> + if (can_native_interpret_type_p (var_type))
> + init = native_interpret_expr (var_type, buf, total_bytes);
> + else
> + {
> + tree itype = build_nonstandard_integer_type (total_bytes * 8, 1);
Shouldn't that 8 be BITS_PER_UNIT ?
I know we have tons of problems with BITS_PER_UNIT is not 8, but adding
further ones is unnecessary.
Jakub
On Thu, 16 Sep 2021, Jakub Jelinek wrote:
> On Thu, Sep 16, 2021 at 11:48:49AM +0200, Richard Biener via Gcc-patches wrote:
> > 2021-09-16 Richard Biener <rguenther@suse.de>
> >
> > PR middle-end/102360
> > * internal-fn.c (expand_DEFERRED_INIT): Make pattern-init
> > of non-memory more robust.
> >
> > * g++.dg/pr102360.C: New testcase.
> > + if (can_native_interpret_type_p (var_type))
> > + init = native_interpret_expr (var_type, buf, total_bytes);
> > + else
> > + {
> > + tree itype = build_nonstandard_integer_type (total_bytes * 8, 1);
>
> Shouldn't that 8 be BITS_PER_UNIT ?
> I know we have tons of problems with BITS_PER_UNIT is not 8, but adding
> further ones is unnecessary.
Well, a byte is 8 bits and we do
unsigned HOST_WIDE_INT total_bytes
= tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
if (init_type == AUTO_INIT_PATTERN)
{
unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
memset (buf, INIT_PATTERN_VALUE, total_bytes);
and thus mix host and target here. I suppose it should be instead
unsigned HOST_WIDE_INT total_bytes
= tree_to_uhwi (TYPE_SIZE (var_type)) / (BITS_PER_UNIT / 8);
or so... in this light * 8 for the build_nonstandard_integer_type
use is correct, no? If total_bytes is really _bytes_.
Richard.
On Thu, Sep 16, 2021 at 12:41:20PM +0200, Richard Biener wrote:
> On Thu, 16 Sep 2021, Jakub Jelinek wrote:
>
> > On Thu, Sep 16, 2021 at 11:48:49AM +0200, Richard Biener via Gcc-patches wrote:
> > > 2021-09-16 Richard Biener <rguenther@suse.de>
> > >
> > > PR middle-end/102360
> > > * internal-fn.c (expand_DEFERRED_INIT): Make pattern-init
> > > of non-memory more robust.
> > >
> > > * g++.dg/pr102360.C: New testcase.
> > > + if (can_native_interpret_type_p (var_type))
> > > + init = native_interpret_expr (var_type, buf, total_bytes);
> > > + else
> > > + {
> > > + tree itype = build_nonstandard_integer_type (total_bytes * 8, 1);
> >
> > Shouldn't that 8 be BITS_PER_UNIT ?
> > I know we have tons of problems with BITS_PER_UNIT is not 8, but adding
> > further ones is unnecessary.
>
> Well, a byte is 8 bits and we do
>
> unsigned HOST_WIDE_INT total_bytes
> = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>
> if (init_type == AUTO_INIT_PATTERN)
> {
> unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> memset (buf, INIT_PATTERN_VALUE, total_bytes);
>
> and thus mix host and target here. I suppose it should be instead
>
> unsigned HOST_WIDE_INT total_bytes
> = tree_to_uhwi (TYPE_SIZE (var_type)) / (BITS_PER_UNIT / 8);
>
> or so... in this light * 8 for the build_nonstandard_integer_type
> use is correct, no? If total_bytes is really _bytes_.
Typically for the native_interpret/native_encode we punt if
BITS_PER_UNIT != 8 || CHAR_BIT != 8 because nobody had the energy
to deal with the weird platforms (especially if we have currently
none, I believe dsp16xx that had 16-bit bytes had been removed in 4.0
and c4x that had 32-bit bytes had been removed in 4.3)
- dunno if the DEFERRED_INIT etc. code has those guards or not
and it clearly documents that this code is not ready for other
configurations.
A byte is not necessarily 8 bits, that is just the most common
size for it, and TYPE_SIZE_UNIT is number of BITS_PER_UNIT bit units.
Jakub
On Thu, 16 Sep 2021, Jakub Jelinek wrote:
> On Thu, Sep 16, 2021 at 12:41:20PM +0200, Richard Biener wrote:
> > On Thu, 16 Sep 2021, Jakub Jelinek wrote:
> >
> > > On Thu, Sep 16, 2021 at 11:48:49AM +0200, Richard Biener via Gcc-patches wrote:
> > > > 2021-09-16 Richard Biener <rguenther@suse.de>
> > > >
> > > > PR middle-end/102360
> > > > * internal-fn.c (expand_DEFERRED_INIT): Make pattern-init
> > > > of non-memory more robust.
> > > >
> > > > * g++.dg/pr102360.C: New testcase.
> > > > + if (can_native_interpret_type_p (var_type))
> > > > + init = native_interpret_expr (var_type, buf, total_bytes);
> > > > + else
> > > > + {
> > > > + tree itype = build_nonstandard_integer_type (total_bytes * 8, 1);
> > >
> > > Shouldn't that 8 be BITS_PER_UNIT ?
> > > I know we have tons of problems with BITS_PER_UNIT is not 8, but adding
> > > further ones is unnecessary.
> >
> > Well, a byte is 8 bits and we do
> >
> > unsigned HOST_WIDE_INT total_bytes
> > = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
> >
> > if (init_type == AUTO_INIT_PATTERN)
> > {
> > unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
> > memset (buf, INIT_PATTERN_VALUE, total_bytes);
> >
> > and thus mix host and target here. I suppose it should be instead
> >
> > unsigned HOST_WIDE_INT total_bytes
> > = tree_to_uhwi (TYPE_SIZE (var_type)) / (BITS_PER_UNIT / 8);
> >
> > or so... in this light * 8 for the build_nonstandard_integer_type
> > use is correct, no? If total_bytes is really _bytes_.
>
> Typically for the native_interpret/native_encode we punt if
> BITS_PER_UNIT != 8 || CHAR_BIT != 8 because nobody had the energy
> to deal with the weird platforms (especially if we have currently
> none, I believe dsp16xx that had 16-bit bytes had been removed in 4.0
> and c4x that had 32-bit bytes had been removed in 4.3)
> - dunno if the DEFERRED_INIT etc. code has those guards or not
> and it clearly documents that this code is not ready for other
> configurations.
> A byte is not necessarily 8 bits, that is just the most common
> size for it, and TYPE_SIZE_UNIT is number of BITS_PER_UNIT bit units.
OK, I'll do s/8/BITS_PER_UNIT/ - I also see that we have
int_size_in_bytes returning TYPE_SIZE_UNIT and that TYPE_SIZE_UNIT
is documented to yeild the type size in 'bytes'.
I do believe that we should officially declare hosts with CHAR_BIT != 8
as unsupported and as you say support for targets with BITS_PER_UNIT != 8
is likely bit-rotten.
Richard.
Hello,
On Thu, 16 Sep 2021, Richard Biener via Gcc-patches wrote:
> > Typically for the native_interpret/native_encode we punt if
> > BITS_PER_UNIT != 8 || CHAR_BIT != 8 because nobody had the energy to
> > deal with the weird platforms (especially if we have currently none, I
> > believe dsp16xx that had 16-bit bytes had been removed in 4.0 and c4x
> > that had 32-bit bytes had been removed in 4.3) - dunno if the
> > DEFERRED_INIT etc. code has those guards or not and it clearly
> > documents that this code is not ready for other configurations. A byte
> > is not necessarily 8 bits, that is just the most common size for it,
> > and TYPE_SIZE_UNIT is number of BITS_PER_UNIT bit units.
>
> OK, I'll do s/8/BITS_PER_UNIT/ - I also see that we have
> int_size_in_bytes returning TYPE_SIZE_UNIT and that TYPE_SIZE_UNIT is
> documented to yeild the type size in 'bytes'.
For better or worse GCCs meaning of 'byte' is really 'unit'; I guess most
introductions of the term 'byte' in comments and function names really
came from either carelessness or from people who knew this fact and
thought it obvious that 'byte' of course is the same as 'unit', but not
the same as octet.
FWIW: (for GCC) both mean the smallest naturally addressable piece of
memory (i.e. what you get when you increase an address by 'one'), and that
is not necessarily 8 bit (but anything else is bit-rotten of course).
As modern use of 'byte' tends to actually mean octet, but old use of byte
(and use in GCC) means unit, we probably should avoid the term byte
alltogether in GCC. But that ship has sailed :-/
> I do believe that we should officially declare hosts with CHAR_BIT != 8
> as unsupported and as you say support for targets with BITS_PER_UNIT !=
> 8 is likely bit-rotten.
(And characters are still something else entirely, except on those couple
platforms where chars, units and octets happen to be the same :) )
But yes.
Ciao,
Michael.
@@ -3045,23 +3045,17 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
if (init_type == AUTO_INIT_PATTERN)
{
- tree alt_type = NULL_TREE;
- if (!can_native_interpret_type_p (var_type))
- {
- alt_type
- = lang_hooks.types.type_for_mode (TYPE_MODE (var_type),
- TYPE_UNSIGNED (var_type));
- gcc_assert (can_native_interpret_type_p (alt_type));
- }
-
unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
memset (buf, INIT_PATTERN_VALUE, total_bytes);
- init = native_interpret_expr (alt_type ? alt_type : var_type,
- buf, total_bytes);
- gcc_assert (init);
-
- if (alt_type)
- init = build1 (VIEW_CONVERT_EXPR, var_type, init);
+ if (can_native_interpret_type_p (var_type))
+ init = native_interpret_expr (var_type, buf, total_bytes);
+ else
+ {
+ tree itype = build_nonstandard_integer_type (total_bytes * 8, 1);
+ wide_int w = wi::from_buffer (buf, total_bytes);
+ init = build1 (VIEW_CONVERT_EXPR, var_type,
+ wide_int_to_tree (itype, w));
+ }
}
expand_assignment (lhs, init, false);
new file mode 100644
@@ -0,0 +1,54 @@
+// { dg-do compile }
+// { dg-options "-fno-tree-dse -O1 -ftrivial-auto-var-init=pattern" }
+
+class A;
+template <typename _Tp, int m, int n> class B {
+public:
+ _Tp val[m * n];
+};
+class C {
+public:
+ C(A);
+};
+struct D {
+ D();
+ unsigned long &operator[](int);
+ unsigned long *p;
+};
+class A {
+public:
+ template <typename _Tp, int m, int n> A(const B<_Tp, m, n> &, bool);
+ int rows, cols;
+ unsigned char *data;
+ unsigned char *datastart;
+ unsigned char *dataend;
+ unsigned char *datalimit;
+ D step;
+};
+template <typename _Tp, int m, int n>
+A::A(const B<_Tp, m, n> &p1, bool)
+ : rows(m), cols(n) {
+ step[0] = cols * sizeof(_Tp);
+ datastart = data = (unsigned char *)p1.val;
+ datalimit = dataend = datastart + rows * step[0];
+}
+class F {
+public:
+ static void compute(C);
+ template <typename _Tp, int m, int n, int nm>
+ static void compute(const B<_Tp, m, n> &, B<_Tp, nm, 1> &, B<_Tp, m, nm> &,
+ B<_Tp, n, nm> &);
+};
+D::D() {}
+unsigned long &D::operator[](int p1) { return p[p1]; }
+template <typename _Tp, int m, int n, int nm>
+void F::compute(const B<_Tp, m, n> &, B<_Tp, nm, 1> &, B<_Tp, m, nm> &,
+ B<_Tp, n, nm> &p4) {
+ A a(p4, false);
+ compute(a);
+}
+void fn1() {
+ B<double, 4, 4> b, c, e;
+ B<double, 4, 1> d;
+ F::compute(b, d, c, e);
+}