Re: [PATCH] TARGET_MEM_REF: Set SYMBOL_REFs as base part

[Zdenek Dvorak <rakdver at atrey dot karlin dot mff dot cuni dot cz>]

Hello,

> on S/390 we see a performance regression against gcc 4.0 due to a adverse
> TARGET_MEM_REF produced by create_mem_ref.
> 
> create_mem_ref currently adds a symbol reference to the base part of the mem ref.
> 
> r1 = MEM[base: r2 + &x]
> 
> On S/390 we can't use symbol refs directly in addresses so we end up with:
> 
> r3 = &x + r2
> r1 = [r3]
> 
> The first instruction then would need a reload because S/390 can't load a symbol
> with an additional addent:
> 
> r4 = &x
> r3 = r4 + r2
> r1 = [r3]
> 
> much better would be:
> 
> r1 = MEM[base:&x index:r2]  what would be transformed to:
> 
> r3 = &x
> r1 = [r3 + r2]
> 
> The attached patch uses the index register part of a mem ref (if it is not already in use)
> for the former base part and puts the symbol reference into the base register part.
> 
> This should not cause any trouble on other architectures but fixes a severe performance 
> regression on S/390.
> 
> Bootstrapped on i686, s390 and s390x. No testsuite regressions on s390 and s390x
> i686 is not through yet.
> 
> OK for mainline - if i686 shows no regressions.

note that I cannot approve the patch, but as the person who wrote tree-ssa-address.c,
I think the patch is correct, and indeed a good idea.

Zdenek

> :ADDPATCH SSA:
> 
> Bye,
> 
> -Andreas-
> 
> 
> 2005-09-19  Andreas Krebbel  <krebbel1@de.ibm.com>
> 
> 	* tree-ssa-address.c (create_mem_ref): Put the symbol reference into the
> 	base register if possible.
> 
> 
> Index: gcc/tree-ssa-address.c
> ===================================================================
> --- gcc/tree-ssa-address.c.orig	2005-06-25 04:01:32.000000000 +0200
> +++ gcc/tree-ssa-address.c	2005-09-19 15:25:20.000000000 +0200
> @@ -525,10 +525,18 @@ create_mem_ref (block_stmt_iterator *bsi
>      
>        /* Add the symbol to base, eventually forcing it to register.  */
>        if (parts.base)
> -	parts.base = force_gimple_operand_bsi (bsi,
> -					       build2 (PLUS_EXPR, addr_type,
> -						       parts.base, tmp),
> -					       true, NULL_TREE);
> +	{
> +	  if (parts.index)
> +	    parts.base = force_gimple_operand_bsi (bsi,
> +						   build2 (PLUS_EXPR, addr_type,
> +							   parts.base, tmp),
> +						   true, NULL_TREE);
> +	  else
> +	    {
> +	      parts.index = parts.base;
> +	      parts.base = tmp;
> +	    }
> +	}
>        else
>  	parts.base = tmp;
>        parts.symbol = NULL_TREE;