Re: Fix for the last subreg related ppc regression

[Geoff Keating <geoffk at geoffk dot org>]

Jan Hubicka <jh@suse.cz> writes:

> Hi,
> as I've asked on discuess list week ago, the problem is due to forming subreg like:
> (subreg:QI (reg:HI x) 3).
> The checking code in simplify_subreg then fails, as the subreg is incorrect (especially
> after converting to mem it may result in accessing uninitialized memory).
> 
> The patch bellow changes gen_lowpart to use simplify_gen_subreg to combine subregs,
> the simplify_subreg already contains check to avoid such improper combines.  At the
> moment it refuses to create them, but possibly we may just return const0_rtx, but
> I would do that in spearate patch.
> 
> While looking at it I found other potential problem in gen_lowpart_for_combine,
> that drops subregs on MEM w/o checking whether the SUBREG_BYTE does not offset
> them to other than lowpart.
> 
> Solves the PPC testcase, i386 bootstrap finished, regtesting is in progress,
> OK assuming it passes?

I think this is just fixing a symptom of the underlying problem.

Where is this expression coming from in the first place?  It seems to
be accessing an uninitialised part of the register.

> Thu May 24 13:41:33 CEST 2001  Jan Hubicka  <jh@suse.cz>
> 	* combine.c (gen_lowpart_for_combine): Drop subreg on MEM only
> 	if it is lowpart subreg; use simplify_gen_subreg to generate
> 	subreg.
> 	* emit-rtl.c (gen_lowpart_common): Use simplify_gen_subreg
> 	to combine subregs.
> 
> Index: combine.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/combine.c,v
> retrieving revision 1.205
> diff -c -3 -p -r1.205 combine.c
> *** combine.c	2001/05/22 07:40:24	1.205
> --- combine.c	2001/05/24 11:39:36
> *************** gen_lowpart_for_combine (mode, x)
> *** 9701,9707 ****
>     /* X might be a paradoxical (subreg (mem)).  In that case, gen_lowpart
>        won't know what to do.  So we will strip off the SUBREG here and
>        process normally.  */
> !   if (GET_CODE (x) == SUBREG && GET_CODE (SUBREG_REG (x)) == MEM)
>       {
>         x = SUBREG_REG (x);
>         if (GET_MODE (x) == mode)
> --- 9701,9708 ----
>     /* X might be a paradoxical (subreg (mem)).  In that case, gen_lowpart
>        won't know what to do.  So we will strip off the SUBREG here and
>        process normally.  */
> !   if (GET_CODE (x) == SUBREG && GET_CODE (SUBREG_REG (x)) == MEM
> !       && subreg_lowpart_p (x))
>       {
>         x = SUBREG_REG (x);
>         if (GET_MODE (x) == mode)
> *************** gen_lowpart_for_combine (mode, x)
> *** 9765,9770 ****
> --- 9766,9772 ----
>     else
>       {
>         int offset = 0;
> +       rtx res;
>   
>         if ((WORDS_BIG_ENDIAN || BYTES_BIG_ENDIAN)
>   	  && GET_MODE_SIZE (GET_MODE (x)) > GET_MODE_SIZE (mode))
> *************** gen_lowpart_for_combine (mode, x)
> *** 9776,9782 ****
>   	  if (BYTES_BIG_ENDIAN)
>   	    offset += difference % UNITS_PER_WORD;
>   	}
> !       return gen_rtx_SUBREG (mode, x, offset);
>       }
>   }
>   
> --- 9778,9787 ----
>   	  if (BYTES_BIG_ENDIAN)
>   	    offset += difference % UNITS_PER_WORD;
>   	}
> !       res = simplify_gen_subreg (mode, x, GET_MODE (x), offset);
> !       if (res)
> ! 	return res;
> !       return gen_rtx_CLOBBER (GET_MODE (x), const0_rtx);
>       }
>   }
>   
> Index: emit-rtl.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/emit-rtl.c,v
> retrieving revision 1.177
> diff -c -3 -p -r1.177 emit-rtl.c
> *** emit-rtl.c	2001/05/22 06:29:39	1.177
> --- emit-rtl.c	2001/05/24 11:39:37
> *************** gen_lowpart_common (mode, x)
> *** 794,812 ****
>     else if (GET_CODE (x) == SUBREG
>   	   && (GET_MODE_SIZE (mode) <= UNITS_PER_WORD
>   	       || GET_MODE_SIZE (mode) == GET_MODE_UNIT_SIZE (GET_MODE (x))))
> !     {
> !       int final_offset;
> ! 
> !       if (GET_MODE (SUBREG_REG (x)) == mode && subreg_lowpart_p (x))
> ! 	return SUBREG_REG (x);
> ! 
> !       /* When working with SUBREGs the rule is that the byte
> ! 	 offset must be a multiple of the SUBREG's mode.  */
> !       final_offset = SUBREG_BYTE (x) + offset;
> !       final_offset = (final_offset / GET_MODE_SIZE (mode));
> !       final_offset = (final_offset * GET_MODE_SIZE (mode));
> !       return gen_rtx_SUBREG (mode, SUBREG_REG (x), final_offset);   
> !     }
>     else if (GET_CODE (x) == REG)
>       {
>         /* Hard registers are done specially in certain cases.  */  
> --- 794,800 ----
>     else if (GET_CODE (x) == SUBREG
>   	   && (GET_MODE_SIZE (mode) <= UNITS_PER_WORD
>   	       || GET_MODE_SIZE (mode) == GET_MODE_UNIT_SIZE (GET_MODE (x))))
> !     return simplify_gen_subreg (mode, x, GET_MODE (x), offset);
>     else if (GET_CODE (x) == REG)
>       {
>         /* Hard registers are done specially in certain cases.  */  

-- 
- Geoffrey Keating <geoffk@geoffk.org>