diff mbox

expr.c: don't assume MUL for scaling pointers

Message ID 201111030323.pA33Nm9W029413@greed.delorie.com
State New
Headers show

Commit Message

DJ Delorie Nov. 3, 2011, 3:23 a.m. UTC
GCC assumes the target has a multiply insn, but better code is
generated using shifts if it doesn't (vs a libcall).  Found with the
rl78-elf port.

	* expr.c (expand_expr_real_2): Don't try to emit a MUL-based
	expression if the target doesn't have a multiply pattern.  Fall
	back to shifts instead of using libgcc calls.

Comments

Hans-Peter Nilsson Nov. 3, 2011, 3:35 a.m. UTC | #1
On Wed, 2 Nov 2011, DJ Delorie wrote:
>
> GCC assumes the target has a multiply insn, but better code is
> generated using shifts if it doesn't (vs a libcall).  Found with the
> rl78-elf port.
>
> 	* expr.c (expand_expr_real_2): Don't try to emit a MUL-based
> 	expression if the target doesn't have a multiply pattern.  Fall
> 	back to shifts instead of using libgcc calls.

Wouldn't this stop targets that have a scaling "lea"-type
indexing pattern but no multiply insns from using those
patterns?

>
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c	(revision 180758)
> +++ gcc/expr.c	(working copy)
> @@ -8289,12 +8289,13 @@ expand_expr_real_2 (sepops ops, rtx targ
>  	}
>
>        /* Attempt to return something suitable for generating an
>  	 indexed address, for machines that support that.  */
>
>        if (modifier == EXPAND_SUM && mode == ptr_mode
> +	  && optab_handler (smul_optab, mode) != CODE_FOR_nothing
>  	  && host_integerp (treeop1, 0))
>  	{
>  	  tree exp1 = treeop1;
>
>  	  op0 = expand_expr (treeop0, subtarget, VOIDmode,
>  			     EXPAND_SUM);
>
Richard Henderson Nov. 3, 2011, 8:19 p.m. UTC | #2
On 11/02/2011 08:23 PM, DJ Delorie wrote:
> GCC assumes the target has a multiply insn, but better code is
> generated using shifts if it doesn't (vs a libcall).  Found with the
> rl78-elf port.
> 
> 	* expr.c (expand_expr_real_2): Don't try to emit a MUL-based
> 	expression if the target doesn't have a multiply pattern.  Fall
> 	back to shifts instead of using libgcc calls.
> 
> Index: gcc/expr.c
> ===================================================================
> --- gcc/expr.c	(revision 180758)
> +++ gcc/expr.c	(working copy)
> @@ -8289,12 +8289,13 @@ expand_expr_real_2 (sepops ops, rtx targ
>  	}
>  
>        /* Attempt to return something suitable for generating an
>  	 indexed address, for machines that support that.  */
>  
>        if (modifier == EXPAND_SUM && mode == ptr_mode
> +	  && optab_handler (smul_optab, mode) != CODE_FOR_nothing

This is the wrong place for this.  With EXPAND_SUM we're expecting
to generate something that is applicable to addresses, and the
addressing form is canonically (plus (mult x n) y).

This does suggest, though, that something ought to be using
expand_mult and not expand_simple_binop.

It's not immediately obvious what that something is, since my first
guess was force_operand and it *does* use expand_mult.


r~
Paul Koning Nov. 8, 2011, 7:19 p.m. UTC | #3
> 
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of DJ Delorie
> Sent: Wednesday, November 02, 2011 11:24 PM
> To: gcc-patches@gcc.gnu.org
> Subject: expr.c: don't assume MUL for scaling pointers
> 
> 
> GCC assumes the target has a multiply insn, but better code is generated using shifts if it doesn't (vs a libcall).  Found with the rl78-elf port.
> 
> 	* expr.c (expand_expr_real_2): Don't try to emit a MUL-based
> 	expression if the target doesn't have a multiply pattern.  Fall
> 	back to shifts instead of using libgcc calls.

Does this also work if the target has a multiply pattern but that is only enabled some of the time (for example, depending on the specified -march=xyz value)?  What about the case where mul is available but much higher cost than shift?

	paul
diff mbox

Patch

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 180758)
+++ gcc/expr.c	(working copy)
@@ -8289,12 +8289,13 @@  expand_expr_real_2 (sepops ops, rtx targ
 	}
 
       /* Attempt to return something suitable for generating an
 	 indexed address, for machines that support that.  */
 
       if (modifier == EXPAND_SUM && mode == ptr_mode
+	  && optab_handler (smul_optab, mode) != CODE_FOR_nothing
 	  && host_integerp (treeop1, 0))
 	{
 	  tree exp1 = treeop1;
 
 	  op0 = expand_expr (treeop0, subtarget, VOIDmode,
 			     EXPAND_SUM);