On Fri, Sep 20, 2013 at 12:00 PM, bin.cheng <bin.ch...@arm.com> wrote:
> Hi,
> For now IVOPT constructs scaled address expression in the form of
> "scaled*index" and checks whether backend supports it. The problem is the
> address expression is invalid on ARM, causing scaled expression disabled in
> IVOPT on ARM. This patch fixes the IVOPT part by constructing rtl address
> expression like "index*scaled+base".
- addr = gen_rtx_fmt_ee (MULT, address_mode, reg1, NULL_RTX);
+ /* Construct address expression in the canonical form of
+ "base+index*scale" and call memory_address_addr_space_p
+ to see whether it is allowed by target processors. */
+ index = gen_rtx_fmt_ee (MULT, address_mode, reg1, NULL_RTX);
for (i = -MAX_RATIO; i <= MAX_RATIO; i++)
{
- XEXP (addr, 1) = gen_int_mode (i, address_mode);
+ if (i == 1)
+ continue;
+
+ XEXP (index, 1) = gen_int_mode (i, address_mode);
+ addr = gen_rtx_fmt_ee (PLUS, address_mode, index, reg1);
if (memory_address_addr_space_p (mode, addr, as))
bitmap_set_bit (valid_mult, i + MAX_RATIO);
so you now build reg1*scale+reg1 - which checks if offset and scale
work at the same time (and with the same value - given this is
really reg1*(scale+1)). It might be that this was implicitely assumed
(or that no targets allow scale but no offset), but it's a change that
requires audit of behavior on more targets.
The above also builds more RTX waste which you can fix by re-using
the PLUS by building it up-front similar to the multiplication. You also
miss the opportunity to have scale == 1 denote as to whether reg1 + reg2
is valid. I would expect that many targets support reg1 * scale +
constant-offset
but not many reg1 * scale + reg2.
So no, the helper now checks sth completely different. What's the problem
with arm supporting reg1 * scale? Why shouldn't it being able to handle
the implicit zero offset?
Richard.
> Hi Richard,
> I thought about the suggestion constructing TARGET_MEM[.] and adding new
> target hook to check whether backend supports such target memory accesses,
> but still want to give this patch a try because:
> 1) RTL pattern "index*scaled+base" is some kind of canonical form of scaled
> address expression and it works fine.
> 2) It won't save us any inconvenience by constructing TARGET_MEM node, on
> contrary, we have to add new target hook checking whether scaled addressing
> mode is supported, which in essence is nothing else than current
> implementation.
>
> Also "base+index*scaled" is re-structured to canonical form
> "index*scaled+base", I constructed the latter form in patch.
> Bootstrapped and tested on x86_64 and arm_a15. Is it OK?
>
> Thanks.
> bin
>
>
> 2013-09-20 Bin Cheng <bin.ch...@arm.com>
>
> * tree-ssa-loop-ivopts.c (multiplier_allowed_in_address_p):
> Construct canonical scaled rtl address expression.
