More info on alpha bug

More info on alpha bug

[H.J. Lu]

As I reported earlier, this code fails to run on alpha/linux:

# gcc -O inline.cc
# a.out
Aborted (core dumped)

The problem is the way how gcc works on alpha. Although alpha is
64 bit, gcc still uses 32 bit operations on many 64 bit integers.
It converts them back and forth between 32 bit/64 bit. As the
result, the return value of slen () is computed as 32 bit and sign
extened to 64 bit upon return. But between inlining and 32 bit/64 bit
coverting, srclen is stored as 32 bit integer on stack. Since
the original value is -1 and the computed value is 5, we get
0xffffffff00000005 instead of 0x0000000000000005. I hope it
will be fixed in the first release.


-- 
H.J. Lu (hjl@gnu.ai.mit.edu)
---
#include <new.h>

typedef struct
{
  unsigned short    len;
  unsigned short    sz;
  char s[1];
} Rep;

inline static void ncopy0(const char* from, char* to, int n)
{
  if (from != to)
  {
    while (--n >= 0) *to++ = *from++;
    *to = 0;
  }
  else
    to[n] = 0;
}

inline static int slen(const char* t) // inline  strlen               
{
  if (t == 0)                                                      
    return 0;                                                       
  else        
  {
    const char* a = t; 
    while (*a++ != 0);       
    return a - 1 - t;
  }
}

inline static Rep* Snew(int newsiz)
{
  if (newsiz == -1)
    abort ();

  Rep* rep = new (operator new (sizeof(Rep) + newsiz)) Rep;
  rep->sz = newsiz;
  return rep;
}

Rep *
copy (Rep *old, const char *src, int srclen, int newlen)
{
  if (srclen < 0) srclen = slen (src);
  if (newlen < srclen) newlen = srclen;
  Rep* rep;
  if (old == 0 || newlen > old->sz)  
  {
    rep = Snew (newlen);
  }
  else
    rep = old;

  rep->len = newlen;
  ncopy0(src, rep->s, srclen);

  return rep;
}

main ()
{
  Rep *rep = copy (0, "Hello", -1, -1);

  exit (0);
}

Re: More info on alpha bug

[Richard Henderson]

On Mon, Oct 20, 1997 at 04:46:32PM -0700, H.J. Lu wrote:
> The problem is the way how gcc works on alpha. Although alpha is
> 64 bit, gcc still uses 32 bit operations on many 64 bit integers.
> It converts them back and forth between 32 bit/64 bit. As the
> result, the return value of slen () is computed as 32 bit and sign
> extened to 64 bit upon return. But between inlining and 32 bit/64 bit
> coverting, srclen is stored as 32 bit integer on stack.

Ok, I've got this figured out now.  This is actually a follow on bug
to the one fixed by

Sat Mar 15 07:17:12 1997  Richard Henderson  <rth@tamu.edu>

        * reload.h (eliminate_regs): Add STORING arg.
        * reload1.c (eliminate_regs): Likewise.
        (eliminate_regs, case SET): Pass that we are storing to recursive call.
        (eliminate_regs, case SUBREG): If storing and same number of words,
        use larger mode.
        * caller-save.c, dbxout.c, dwarfout.c, dwarf2out.c, reload.c, sdbout.c:
        Change all calls to eliminate_regs.

(I also found the test case for this one, if anyone is interested.)

In the previous example, reload lost track of the width of the significant
data in the pseudo, spilled as SI, reloaded as DI.  The solution I came
to with Kenner at the time is to always spill full registers.

In this example, the above code kicks in and widens the destination of
the store, but the source stays in SImode.  This leads to 

  (set (mem:DI (plus (reg $30) (const_int 120)))
       (reg:SI $1)))

which somehow fails to choke the backend and yields an SImode store in
the assembly.

My solution, then, is to notice when the original DEST and eliminated
DEST differ in mode, and if they do, emit a (possibly paradoxical) 
subreg of SRC. 

I don't know if this is better or worse than adjusting the mode of SRC
directly, but I have the idea that the size of references to registers
should be consistent one to the next, with subreg thrown in if necessary
to hide the differences.


r~

Re: More info on alpha bug

[Jim Wilson]

	Mon Oct 27 18:18:00 1997  Richard Henderson  <rth@cygnus.com>

	* reload1.c (eliminate_regs [SET]): If [SUBREG] widened the mode of
	DEST for the spill, adjust mode of SRC to compensate.

This patch looks OK to me.

	In this example, the above code kicks in and widens the destination of
	the store, but the source stays in SImode.  This leads to 

	  (set (mem:DI (plus (reg $30) (const_int 120)))
	       (reg:SI $1)))

	which somehow fails to choke the backend and yields an SImode store in
	the assembly.

Yes, that is wierd.  I looked at it out of curiousity.  It can fail only
if we try to recognize the instruction.  However, the instruction had already
been recognized (as movdf+1) before reload ran, and reload does not bother to
re-recognize an instruction when performing simple reloads on them, hence we
never notice the problem.

	I don't know if this is better or worse than adjusting the mode of SRC
	directly, but I have the idea that the size of references to registers
	should be consistent one to the next, with subreg thrown in if necessary
	to hide the differences.

Pseudos are represented by a unique RTX, and hence have a fixed mode.  If
you need a different mode for a pseudo, you must use a SUBREG.  Hard registers
can be represented by multiple RTXes, and can have any mode allowed by
HARD_REGNO_MODE_OK.  If you have a hard register, then creating a SUBREG is
not necessary.  Creating a SUBREG for a hard register should be harmless
though.

Jim

Re: More info on alpha bug

[Richard Henderson]

On Wed, Sep 23, 1998 at 08:43:29AM -0700, H.J. Lu wrote:
> It seems that the bug is in alpha_align_insns. There is

Oops.  I fixed that Monday but forgot to check it in.


r~

More info on alpha bug

[H.J. Lu]

Hi, Richard,

It seems that the bug is in alpha_align_insns. There is

     else if (ofs + len > align)
       {
	  int nop_count = (align - ofs) / 4;

	  .....
          do
            emit_insn_before ((*next_nop)(&prev_in_use), where); 
          while (--nop_count);

But it doesn't handle the cae of nop_count == 0. In that case,
ofs == align == 8 and len == 4.


-- 
H.J. Lu (hjl@gnu.org)