This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH]Construct canonical scaled address expression in IVOPT



> -----Original Message-----
> From: Richard Biener [mailto:richard.guenther@gmail.com]
> Sent: Monday, September 23, 2013 8:08 PM
> To: Bin Cheng
> Cc: GCC Patches
> Subject: Re: [PATCH]Construct canonical scaled address expression in IVOPT
> 
> On Fri, Sep 20, 2013 at 12:00 PM, bin.cheng <bin.cheng@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.
So literally, function multiplier_allowed_in_address_p should check whether
multiplier is allowed in any kind of addressing mode, like [reg*scale +
offset] and [reg*scale + reg].   Apparently it's infeasible to check every
possibility for each architecture, is it ok we at least check "index", then
"addr" if "index" is failed?  By "any kind of addressing modes", I mean
modes supported by function get_address_cost,  i.e., in form of "[base] +
[off] + [var] + (reg|reg*scale)".

> 
> 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
Yes, this can be fixed.

> 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.
I thought scale==1 is unnecessary because the addressing mode degrades into
"reg" or "reg+reg".  Moreover, calls of multiplier_allowed_in_address_p in
both get_address_cost and get_computation_cost_at have scale other than 1.

> 
> 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?

As Richard clarified, ARM does not support scaled addressing mode without
base register.

Thanks.
bin




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]