This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Unreviewed convert_modes()/TRULY_NOOP_TRUNCATION patch


Just a reminder that this patch hasn't been reviewed.

Richard Sandiford <rsandifo@redhat.com> writes:
> This patch deals with a bad interaction between convert_modes() and
> TRULY_NOOP_TRUNCATION.  It fixes execute/920428-1.c for mips/n64
> and execute/longlong.c for mipsisa64-elf.
> 
> The problem is this check in convert_modes():
> 
>   /* We can do this with a gen_lowpart if both desired and current modes
>      are integer, and this is either a constant integer, a register, or a
>      non-volatile MEM.  Except for the constant case where MODE is no
>      wider than HOST_BITS_PER_WIDE_INT, we must be narrowing the operand.  */
> 
>   if ((GET_CODE (x) == CONST_INT
>        && GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT)
>       || (GET_MODE_CLASS (mode) == MODE_INT
> 	  && GET_MODE_CLASS (oldmode) == MODE_INT
> 	  && (GET_CODE (x) == CONST_DOUBLE
> 	      || (GET_MODE_SIZE (mode) <= GET_MODE_SIZE (oldmode)
> 		  && ((GET_CODE (x) == MEM && ! MEM_VOLATILE_P (x)
> 		       && direct_load[(int) mode])
> 		      || (GET_CODE (x) == REG
> 			  && (! HARD_REGISTER_P (x)
> 			      || HARD_REGNO_MODE_OK (REGNO (x), mode))
> 			  && TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
> 						    GET_MODE_BITSIZE (GET_MODE (x)))))))))
>     {
> 
> ...and in particular the fact that TRULY_NOOP_TRUNCATION is only checked
> for registers.  The assumption seems to be that gen_lowpart() will return
> a MEM if given a MEM.  But that isn't always true:
> 
>   else if (GET_CODE (x) == MEM)
>     {
>       /* The only additional case we can do is MEM.  */
>       int offset = 0;
> 
>       /* The following exposes the use of "x" to CSE.  */
>       if (GET_MODE_SIZE (GET_MODE (x)) <= UNITS_PER_WORD
> 	  && SCALAR_INT_MODE_P (GET_MODE (x))
> 	  && ! no_new_pseudos)
> 	return gen_lowpart (mode, force_reg (GET_MODE (x), x));
> 
> The two obvious alternatives are:
> 
>   (1) Make the TRULY_NOOP_TRUNCATION check cover both MEMs and REGs.
> 
>   (2) Disable the force_reg optimisation if the register lowpart
>       isn't a true nop truncation.
> 
> So suppose we have a 64-bit mips target and we're converting a DImode
> MEM into an SImode value.  If we go with (1), convert_modes() will punt
> to convert_move(), which in turn will use the trunc* patterns.  Since
> those patterns don't support mem->reg truncations, it will force the
> MEM into a register first.  This leads to code like:
> 
>     ld $4,foo
>     dsll32 $5,$4,0
> 
> (2) means that convert_mode() will return an SImode MEM, so (on a
> big-endian target) the code would look like:
> 
>     lw $4,foo+4
> 
> The code for (2) is obviously better when only the low part of foo is
> used.  In many (but not all) cases, it's also better when the high part
> or full value is used as well.  From that POV, I'd obviously prefer to
> go with (2).
> 
> Tested on mips64{,el}-linux-gnu, mipsisa64-elf, mips64vre-elf, etc.
> OK to install?
> 
> Richard
> 
> 
> 	* emit-rtl.c (gen_lowpart): Don't force MEMs into a register unless
> 	the register lowpart is a TRULY_NOOP_TRUNCATION.
> 
> Index: emit-rtl.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/emit-rtl.c,v
> retrieving revision 1.352
> diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.352 emit-rtl.c
> *** emit-rtl.c	4 Nov 2003 09:14:18 -0000	1.352
> --- emit-rtl.c	7 Nov 2003 19:59:28 -0000
> *************** gen_lowpart (enum machine_mode mode, rtx
> *** 1356,1361 ****
> --- 1356,1363 ----
>         /* The following exposes the use of "x" to CSE.  */
>         if (GET_MODE_SIZE (GET_MODE (x)) <= UNITS_PER_WORD
>   	  && SCALAR_INT_MODE_P (GET_MODE (x))
> + 	  && TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
> + 				    GET_MODE_BITSIZE (GET_MODE (x)))
>   	  && ! no_new_pseudos)
>   	return gen_lowpart (mode, force_reg (GET_MODE (x), x));
>   


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]