On Sun, Nov 11, 2012 at 7:01 AM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> This patch also handles SIGN_EXTEND. Tested on Linux/x32. OK to
>> install?
>
> I'd cautious here, that's uncharted territory and the SIGN_EXTEND case isn't
> covered by your testing.
>
>> 2012-11-10 H.J. Lu <hongjiu...@intel.com>
>>
>> PR middle-end/55247
>> PR middle-end/55259
>> * emit-rtl.c (adjust_address_1): Handle ZERO_EXTEND and
>> SIGN_EXTEND.
>
> The new transformation is:
> (zero_extend ADDR) + CONST_INT -> (zero_extend (ADDR + CONST_INT))
> but in convert_memory_address_addr_space we do the opposite:
> (zero_extend (ADDR + CONST_INT)) -> (zero_extend ADDR) + CONST_INT
>
> Which one is the canonical form in the end?
>
>> @@ -2109,6 +2111,20 @@ adjust_address_1 (rtx memref, enum machine_mode mode,
>> HOST_WIDE_INT offset, addr = gen_rtx_LO_SUM (address_mode, XEXP (addr, 0),
>> plus_constant (address_mode,
>> XEXP (addr, 1), offset));
>> + /* We permute zero/sign-extension and addition operation only if
>> + converting the constant does not change it. */
>> + else if ((GET_CODE (addr) == ZERO_EXTEND
>> + || GET_CODE (addr) == SIGN_EXTEND)
>> + && (x = GEN_INT (offset),
>> + x == convert_memory_address_addr_space (address_mode,
>> + x,
>> + attrs.addrspace)))
>> + {
>> + enum rtx_code code = GET_CODE (addr);
>> + addr = XEXP (addr, 0);
>> + addr = plus_constant (GET_MODE (addr), addr, offset);
>> + addr = gen_rtx_fmt_e (code, address_mode, addr);
>> + }
>
> You need to test the truncation here, not the extension. Moreover, the
> transformation is valid only under a no-overflow assumption, so I think we
> need to explicitly request pointer_mode (or else restrict the transformation
> to small offsets).
>
>> diff --git a/gcc/recog.c b/gcc/recog.c
>> index ee68e30..a916ef6 100644
>> --- a/gcc/recog.c
>> +++ b/gcc/recog.c
>> @@ -1934,15 +1934,22 @@ int
>> offsettable_address_addr_space_p (int strictp, enum machine_mode mode, rtx
>> y, addr_space_t as)
>> {
>> - enum rtx_code ycode = GET_CODE (y);
>> + enum rtx_code ycode;
>> rtx z;
>> - rtx y1 = y;
>> + rtx y1;
>> rtx *y2;
>> int (*addressp) (enum machine_mode, rtx, addr_space_t) =
>> (strictp ? strict_memory_address_addr_space_p
>>
>> : memory_address_addr_space_p);
>>
>> unsigned int mode_sz = GET_MODE_SIZE (mode);
>>
>> + /* Allow zero-extended or sign-extended address. */
>> + if (GET_CODE (y) == ZERO_EXTEND || GET_CODE (y) == SIGN_EXTEND)
>> + y = XEXP (y, 0);
>> +
>> + ycode = GET_CODE (y);
>> + y1 = y;
>> +
>> if (CONSTANT_ADDRESS_P (y))
>> return 1;
>
> That's not fully correct since you don't test the final form of the address.
> You instead need to duplicate the adjust_address_1 change here:
>
> /* The offset added here is chosen as the maximum offset that
> any instruction could need to add when operating on something
> of the specified mode. We assume that if Y and Y+c are
> valid addresses then so is Y+d for all 0<d<c. adjust_address will
> go inside a LO_SUM here, so we do so as well. */
> if (GET_CODE (y) == LO_SUM
> && mode != BLKmode
> && mode_sz <= GET_MODE_ALIGNMENT (mode) / BITS_PER_UNIT)
> z = gen_rtx_LO_SUM (GET_MODE (y), XEXP (y, 0),
> plus_constant (GET_MODE (y), XEXP (y, 1),
> mode_sz - 1));
> else
> z = plus_constant (GET_MODE (y), y, mode_sz - 1);
>
>
> Adjusted patch attached.
>
> --
> Eric Botcazou
It fixes the problem. Can you check it in?
Thanks.
--
H.J.
