Re: Reload bug & SRA oddness

[Bernd Schmidt <bernds_cb1 at t-online dot de>]

Alexandre Oliva wrote:
> On Apr 20, 2007, Bernd Schmidt <bernds_cb1@t-online.de> wrote:
> 
>> Bootstrap completed, and no check-gcc regressions on i686-linux.  Can
>> you send me a testcase that fails when this change is made?
> 
> Native configuration is x86_64-pc-linux-gnu
> FAIL: gcc.c-torture/execute/20040709-2.c execution,  -O3 -g 

Okay, I've set up an x86-64 install and debugged this a little further.
 I've attached a reduced testcase for this.

What's happening is that in esra, the following happens to function retmeA:
retmeA (x)                              retmeA (x)
{                                       {
                                  >       <unnamed-unsigned:11> x$k;
                                  >       unsigned char x$B0F5;
  struct A D.2000;                        struct A D.2000;

<bb 2>:                                 <bb 2>:
  D.2000 = x;                     |       x$k_1 = x.k;
                                  >       x$B0F5_2 = BIT_FIELD_REF <x,
                                  >       D.2000.k = x$k_1;
                                  >       BIT_FIELD_REF <D.2000, 5, 0>
  return D.2000;                          return D.2000;

}                                       }

It is then inlined into fn1A, where esra runs again, and when scanning
the function, we run into the new BIT_FIELD_REFs.  When it tries to deal
with the following stmt:

   BIT_FIELD_REF <D.4935, 5, 0> = BIT_FIELD_REF <x$B0F5_11, 5, 0>;

it just calls scalarize_use with D.4935 and is_output==true, causing us
to insert two additional stmts:

+  D.4935.k = SR.282_16;
   BIT_FIELD_REF <D.4935, 5, 0> = BIT_FIELD_REF <x$B0F5_11, 5, 0>;
+  SR.282_17 = D.4935.k;

Both of these are unnecessary (and inefficient), as the BIT_FIELD_REF
does not touch "k".  With my patch to restore is_output behaviour, we
only eliminate the first, but not the second.

IMO the real problem is that SRA doesn't know how to deal with these
BIT_FIELD_REFs properly - possibly something similar to how we deal with
ARRAY_RANGE_REF could be used?  I'll keep investigating this, but for
the moment there are two more points I'd like to make:

 * 20040709-2.c (which is very bitfield intensive) at -O3 has a pretty
large code size regression at least on x86-64 when compared to before
your SRA patch.
 * I've seen no serious attempt to investigate or fix the regressions
Andrew Pinski has reported.

As a consequence of all these problems, I'd like to see this patch
reverted for now until it we know how to do it properly.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif

extern void abort (void);
extern void exit (int);

unsigned int
myrnd (void)
{
  static unsigned int s = 1388815473;
  s *= 1103515245;
  s += 12345;
  return (s / 65536) % 2048;
}

struct __attribute__ ((packed)) A
{
  unsigned short i:1, l:1, j:3, k:11;
};
struct A sA;
struct A
retmeA (struct A x)
{
  return x;
}
unsigned int
fn1A (unsigned int x)
{
  struct A y = sA;
  y.k += x;
  y = retmeA (y);
  return y.k;
}
unsigned int
fn2A (unsigned int x)
{
  struct A y = sA;
  y.k += x;
  y.k %= 15;
  return y.k;
}
unsigned int
retitA (void)
{
  return sA.k;
}
unsigned int
fn3A (unsigned int x)
{
  sA.k += x;
  return retitA ();
}

void
testA (void)
{
  int i;
  unsigned int mask, v, a, r;
  struct A x;
  char *p = (char *) &sA;
  for (i = 0; i < sizeof (sA); ++i)
    *p++ = myrnd ();
  if (__builtin_classify_type (sA.l) == 8)
    sA.l = 5.25;
  sA.k = -1;
  mask = sA.k;
  v = myrnd ();
  a = myrnd ();
  sA.k = v;
  x = sA;
  r = fn1A (a);
  if (x.i != sA.i || x.j != sA.j || x.k != sA.k || x.l != sA.l
      || ((v + a) & mask) != r)
    abort ();
  v = myrnd ();
  a = myrnd ();
  sA.k = v;
  x = sA;
  r = fn2A (a);
  if (x.i != sA.i || x.j != sA.j || x.k != sA.k || x.l != sA.l
      || ((((v + a) & mask) % 15) & mask) != r)
    abort ();
  v = myrnd ();
  a = myrnd ();
  sA.k = v;
  x = sA;
  r = fn3A (a);
  if (x.i != sA.i || x.j != sA.j || sA.k != r || x.l != sA.l
      || ((v + a) & mask) != r)
    abort ();
}

int
main (void)
{
  testA ();
  exit (0);
}