Re: [cft] subreg validation patch

[Ulrich Weigand <uweigand at de dot ibm dot com>]

Richard Henderson wrote:

>Could everyone please try the following on their favorite platform
>and report back with new failures?

I've tried your patch on s390-ibm-linux and s390x-ibm-linux.  On both
platforms, bootstrap fails when building stage2, because of a bug in
regmove_optimize.  The code there tries to convert a paradoxical
subreg move into a move with the opposite subreg on the other side,
but completely messes up the subreg offset.  The following patch
appears to fix that problem:

Index: gcc/regmove.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/regmove.c,v
retrieving revision 1.164
diff -c -p -r1.164 regmove.c
*** gcc/regmove.c	9 Oct 2004 19:19:24 -0000	1.164
--- gcc/regmove.c	14 Nov 2004 02:56:01 -0000
*************** regmove_optimize (rtx f, int nregs, FILE
*** 1150,1159 ****
  		  && GET_MODE_SIZE (GET_MODE (dst))
  		     >= GET_MODE_SIZE (GET_MODE (SUBREG_REG (dst))))
  		{
- 		  src_subreg
- 		    = gen_rtx_SUBREG (GET_MODE (SUBREG_REG (dst)),
- 				      src, SUBREG_BYTE (dst));
  		  dst = SUBREG_REG (dst);
  		}
  	      if (!REG_P (dst)
  		  || REGNO (dst) < FIRST_PSEUDO_REGISTER)
--- 1150,1157 ----
  		  && GET_MODE_SIZE (GET_MODE (dst))
  		     >= GET_MODE_SIZE (GET_MODE (SUBREG_REG (dst))))
  		{
  		  dst = SUBREG_REG (dst);
+ 		  src_subreg = gen_lowpart (GET_MODE (dst), src);
  		}
  	      if (!REG_P (dst)
  		  || REGNO (dst) < FIRST_PSEUDO_REGISTER)


With that patch, bootstrap and regtest complete on s390-ibm-linux
without regressions.  On s390x, however, I get a couple of 
regressions:
> FAIL: gcc.c-torture/execute/20040709-1.c compilation,  -O1
> UNRESOLVED: gcc.c-torture/execute/20040709-1.c execution,  -O1
> FAIL: gcc.c-torture/execute/20040709-1.c compilation,  -O2
> UNRESOLVED: gcc.c-torture/execute/20040709-1.c execution,  -O2
> FAIL: gcc.c-torture/execute/930930-2.c compilation,  -O1
> UNRESOLVED: gcc.c-torture/execute/930930-2.c execution,  -O1
> FAIL: gcc.c-torture/execute/930930-2.c compilation,  -O2
> UNRESOLVED: gcc.c-torture/execute/930930-2.c execution,  -O2
> FAIL: gcc.c-torture/execute/930930-2.c compilation,  -O3 -fomit-frame-pointer
> UNRESOLVED: gcc.c-torture/execute/930930-2.c execution,  -O3 -fomit-frame-pointer
> FAIL: gcc.c-torture/execute/930930-2.c compilation,  -O3 -g
> UNRESOLVED: gcc.c-torture/execute/930930-2.c execution,  -O3 -g
> FAIL: gcc.c-torture/execute/930930-2.c compilation,  -Os
> UNRESOLVED: gcc.c-torture/execute/930930-2.c execution,  -Os
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O1
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O1
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O2
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O2
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O3 -fomit-frame-pointer
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O3 -fomit-frame-pointer
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O3 -fomit-frame-pointer -funroll-loops
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O3 -fomit-frame-pointer -funroll-loops
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -O3 -g
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -O3 -g
> FAIL: gcc.c-torture/execute/stdarg-3.c compilation,  -Os
> UNRESOLVED: gcc.c-torture/execute/stdarg-3.c execution,  -Os

These all have the same reason: this piece of code in store_bit_field

  if (bitpos == 0
      && bitsize == GET_MODE_BITSIZE (fieldmode)
      && (!MEM_P (op0)
          ? ((GET_MODE_SIZE (fieldmode) >= UNITS_PER_WORD
             || GET_MODE_SIZE (GET_MODE (op0)) == GET_MODE_SIZE (fieldmode))
             && byte_offset % GET_MODE_SIZE (fieldmode) == 0)
          : (! SLOW_UNALIGNED_ACCESS (fieldmode, MEM_ALIGN (op0))
             || (offset * BITS_PER_UNIT % bitsize == 0
                 && MEM_ALIGN (op0) % GET_MODE_BITSIZE (fieldmode) == 0))))
    {
      if (GET_MODE (op0) != fieldmode)
        {
          if (GET_CODE (op0) == SUBREG)
            {
              /* Else we've got some float mode source being extracted
                 into a different float mode destination -- this
                 combination of subregs results in Severe Tire
                 Damage.  */
              gcc_assert (GET_MODE (SUBREG_REG (op0)) == fieldmode
                          || GET_MODE_CLASS (fieldmode) == MODE_INT
                          || GET_MODE_CLASS (fieldmode) == MODE_PARTIAL_INT);
              op0 = SUBREG_REG (op0);
            }
          if (REG_P (op0))
            op0 = gen_rtx_SUBREG (fieldmode, op0, byte_offset);
          else
            op0 = adjust_address (op0, fieldmode, offset);
        }
      emit_move_insn (op0, value);
      return value;
    }

calls gen_rtx_SUBREG to form a DFmode subreg of one of the subwords of
a TImode register.

I'm not sure how to fix that, not least because I still don't understand
why this subreg is now supposed to be invalid in the first place ...


One other question about your patch:

+   /* For hard registers, we already have all of these rules collected in
+      subreg_offset_representable_p.  */
+   if (reg && REG_P (reg) && HARD_REGISTER_P (reg))
+     return subreg_offset_representable_p (REGNO (reg), imode, offset, omode);

This check is not quite the same as the check done in simplify_subreg.
In particular, you neither check for REG_CANNOT_CHANGE_MODE_P, nor is
there the special checks for frame-related registers.  While I'm not
quite sure I fully understand those checks, shouldn't the rules be
the same here as in simplify_subreg?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  Linux on zSeries Development
  Ulrich.Weigand@de.ibm.com