Re: [spu] Fix ICEs with complex integer types (4.4 regression)

[trevor_smigiel at playstation dot sony dot com]

Hi Ulrich,

You have identified the real problem is in store_bit_field_1, why aren't
we fixing it there?

At a minimum, we should add a comment to your fix in SPU that says
something like:
   /* The following code should only happen when the inner mode is an
      integer mode, but we cannot rely on the middle end to properly
      enforce CANNOT_CHANGE_MODE, so we handle any type of mode. */

Also, remove the "GET_MODE_CLASS (imode) == MODE_INT" check in the
assert.  It will always be true because we now call int_mode_for_mode.

Trevor

* Ulrich Weigand <uweigand@de.ibm.com> [2009-03-04 05:40]:
> Hello,
> 
> the following patch:
> 
> 2009-01-11  Adam Nemet  <anemet@caviumnetworks.com>
> 
>         * expmed.c (store_bit_field_1): Properly truncate the paradoxical
>         subreg of op0 to the original op0.
> 
> http://gcc.gnu.org/ml/gcc-patches/2008-12/msg00766.html
> 
> exposes a problem on the SPU that causes internal compiler errors in
> several test cases using complex integer function return types.
> 
> The problem is caused by expmed.c:store_bit_field_1 creating a TImode
> subreg of a pseudo of some complex integer type (or the integer type
> of equivalent length).  It seems to me that these subregs are not
> actually valid in the first place, because the SPU CANNOT_CHANGE_MODE
> macro rejects it (because integer values of smaller size do *not*
> occupy the low part of a TImode register on the SPU), however, the
> store_bit_field_1 logic never checks for this.
> 
> Now, the SPU back-end has some logic to cope with those paradoxical
> subregs anyway (the valid_subreg checks in spu_expand_mov, and the
> adjust_operand logic in the insv/extv expanders).  However, the
> patch mentioned above exposes one situation that is currently not
> covered by that logic: the trunctidi expander is called on a 
> source that is a TImode paradoxical subreg of a complex integer
> mode.  This runs into a failed assertion in spu_expand_mov, where
> it assumes that the inner mode must be integral.
> 
> The following patch extends the spu_expand_mov logic to also
> handle non-integral modes, by performing the shift in a same
> size integral mode.  This fixes the test suite ICEs.
> 
> (Longer term, the middle-end should really be fixed to not generate
> those subregs in the first place.  But for now, I think the patch
> below is the conservative fix to get SPU working again ...)
> 
> Tested on spu-elf with no regressions, fixes the following FAILs:
> 
> FAIL: gcc.c-torture/compile/pr27889.c  -O0  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -O0  (test for excess errors)
> FAIL: gcc.c-torture/compile/pr27889.c  -O1  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -O1  (test for excess errors)
> FAIL: gcc.c-torture/compile/pr27889.c  -O2  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -O2  (test for excess errors)
> FAIL: gcc.c-torture/compile/pr27889.c  -O3 -fomit-frame-pointer  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -O3 -fomit-frame-pointer  (test for excess errors)
> FAIL: gcc.c-torture/compile/pr27889.c  -O3 -g  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -O3 -g  (test for excess errors)
> FAIL: gcc.c-torture/compile/pr27889.c  -Os  (internal compiler error)
> FAIL: gcc.c-torture/compile/pr27889.c  -Os  (test for excess errors)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -O0  (internal compiler error)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -O1  (internal compiler error)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -O2  (internal compiler error)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -O3 -fomit-frame-pointer  (internal compiler error)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -O3 -g  (internal compiler error)
> FAIL: gcc.c-torture/execute/20050121-1.c compilation,  -Os  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -O0  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -O1  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -O2  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -O3 -fomit-frame-pointer  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -O3 -g  (internal compiler error)
> FAIL: gcc.c-torture/execute/complex-6.c compilation,  -Os  (internal compiler error)
> FAIL: gcc.dg/compat/scalar-return-3 c_compat_y_tst.o compile,  (internal compiler error)
> FAIL: gcc.dg/compat/scalar-return-4 c_compat_y_tst.o compile,  (internal compiler error)
> FAIL: gcc.dg/fold-mulconj-1.c (internal compiler error)
> FAIL: gcc.dg/fold-mulconj-1.c (test for excess errors)
> 
> OK for mainline?
> 
> Bye,
> Ulrich
> 
> 
> ChangeLog:
> 
> 	* config/spu/spu.c (spu_expand_mov): Handle invalid subregs
> 	of non-integer mode as well.
> 
> 
> Index: gcc/config/spu/spu.c
> ===================================================================
> *** gcc/config/spu/spu.c	(revision 144550)
> --- gcc/config/spu/spu.c	(working copy)
> *************** spu_expand_mov (rtx * ops, enum machine_
> *** 4114,4130 ****
>     if (GET_CODE (ops[1]) == SUBREG && !valid_subreg (ops[1]))
>       {
>         rtx from = SUBREG_REG (ops[1]);
> !       enum machine_mode imode = GET_MODE (from);
>   
>         gcc_assert (GET_MODE_CLASS (mode) == MODE_INT
>   		  && GET_MODE_CLASS (imode) == MODE_INT
>   		  && subreg_lowpart_p (ops[1]));
>   
>         if (GET_MODE_SIZE (imode) < 4)
> ! 	{
> ! 	  from = gen_rtx_SUBREG (SImode, from, 0);
> ! 	  imode = SImode;
> ! 	}
>   
>         if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (imode))
>   	{
> --- 4114,4129 ----
>     if (GET_CODE (ops[1]) == SUBREG && !valid_subreg (ops[1]))
>       {
>         rtx from = SUBREG_REG (ops[1]);
> !       enum machine_mode imode = int_mode_for_mode (GET_MODE (from));
>   
>         gcc_assert (GET_MODE_CLASS (mode) == MODE_INT
>   		  && GET_MODE_CLASS (imode) == MODE_INT
>   		  && subreg_lowpart_p (ops[1]));
>   
>         if (GET_MODE_SIZE (imode) < 4)
> ! 	imode = SImode;
> !       if (imode != GET_MODE (from))
> ! 	from = gen_rtx_SUBREG (imode, from, 0);
>   
>         if (GET_MODE_SIZE (mode) < GET_MODE_SIZE (imode))
>   	{
> -- 
>   Dr. Ulrich Weigand
>   GNU Toolchain for Linux on System z and Cell BE
>   Ulrich.Weigand@de.ibm.com