fixing typo in expr.c to allow proper recognition of complex addresses in some arches.

I stumbled on a case like this:
If the multipliers allowed for addressing modes are dependent on actual
mode, in some cases compiler
refuses to recognize address as legitimate.
It boils down to this place in expr.c where address_mode is incorrectly
used instead of actual mode.

I rebootstraped and tested on x86_64-linux-gnu but there's no testcase to
see the improved behavior...

Please patch the trunk.
Thanks
Igor

On 10/24/13 18:20, Igor Shevlyakov wrote:
> I stumbled on a case like this:
> If the multipliers allowed for addressing modes are dependent on actual
> mode, in some cases compiler
> refuses to recognize address as legitimate.
> It boils down to this place in expr.c where address_mode is incorrectly
> used instead of actual mode.
>
> I rebootstraped and tested on x86_64-linux-gnu but there's no testcase to
> see the improved behavior...
>
> Please patch the trunk.
> Thanks
> Igor
>
> =====
>
> 2013-10-24  Igor
> Shevlyakov<igor.shevlyakov@**gmail.com<igor.shevlyakov@gmail.com>
>>
>
> * expr.c (expand_expr_real_1): Use proper memory access mode instead of
> address_mode
>    in the call to memory_address_addr_space.
Thanks.  Installed on the trunk.

FWIW, the PA might show improvements from this patch.  It's scaled 
indexed addressing modes only allow multipliers that are the same as the 
size of the object being accessed.  ie, a load which accesses 2 bytes 
allows *2, load accessing 4 bytes allows *4 and similarly for double-words.

Anyway, the PA is a dead processor, but if you're looking for ways to 
show problems around limited scaled indexed addressing on a processor 
that is in the trunk, you might want to look at the PA.

jeff

> Thanks.  Installed on the trunk.

Well, no, that will be problematic for some architectures.  The history of 
this piece of code is complicated and it's admittedly lacking a comment, but 
the purpose of the block is clear enough:

	op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_SUM);
	op0 = memory_address_addr_space (address_mode, op0, as);
	if (!integer_zerop (TREE_OPERAND (exp, 1)))
	  {
	    rtx off
	      = immed_double_int_const (mem_ref_offset (exp), address_mode);
	    op0 = simplify_gen_binary (PLUS, address_mode, op0, off);
	  }
	op0 = memory_address_addr_space (mode, op0, as);

The offset computation is done in address_mode and then, only at the end, 
converted to mode.

But it's nothing to do with construction inside
memory_address_addr_space but with validity of an address construct.
first thing memory_address_addr_space does is deconstructing
address_mode from as parameter. But if one doesn't pass actual mode
there's not way to find it and call to legitimate_address_p hook will
fail.

On Fri, Oct 25, 2013 at 1:39 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Thanks.  Installed on the trunk.
>
> Well, no, that will be problematic for some architectures.  The history of
> this piece of code is complicated and it's admittedly lacking a comment, but
> the purpose of the block is clear enough:
>
>         op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_SUM);
>         op0 = memory_address_addr_space (address_mode, op0, as);
>         if (!integer_zerop (TREE_OPERAND (exp, 1)))
>           {
>             rtx off
>               = immed_double_int_const (mem_ref_offset (exp), address_mode);
>             op0 = simplify_gen_binary (PLUS, address_mode, op0, off);
>           }
>         op0 = memory_address_addr_space (mode, op0, as);
>
> The offset computation is done in address_mode and then, only at the end,
> converted to mode.
>
> --
> Eric Botcazou

On 10/25/13 02:39, Eric Botcazou wrote:
>> Thanks.  Installed on the trunk.
>
> Well, no, that will be problematic for some architectures.  The history of
> this piece of code is complicated and it's admittedly lacking a comment, but
> the purpose of the block is clear enough:
>
> 	op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_SUM);
> 	op0 = memory_address_addr_space (address_mode, op0, as);
> 	if (!integer_zerop (TREE_OPERAND (exp, 1)))
> 	  {
> 	    rtx off
> 	      = immed_double_int_const (mem_ref_offset (exp), address_mode);
> 	    op0 = simplify_gen_binary (PLUS, address_mode, op0, off);
> 	  }
> 	op0 = memory_address_addr_space (mode, op0, as);
>
> The offset computation is done in address_mode and then, only at the end,
> converted to mode.
My reading of memory_address_addr_space is that MODE is the mode of the 
memory reference, not the mode of the address.  I fail to see how 
passing in the mode of the address in the first call can be correct.

What am I missing?

jeff