Re: [patch, arm] Fix PR target/50305 (arm_legitimize_reload_address problem)

["Ulrich Weigand" <uweigand@de.ibm.com>]

Ramana Radhakrishnan wrote:
> On 9 September 2011 18:04, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> >
> > In theory, LEGITIMIZE_RELOAD_ADDRESS could attempt to handle them by
> > substituting the equivalent constant and then reloading the result.
> > However, this might need additional steps (pushing to the constant pool,
> > reloading the constant pool address, ...) which would lead to significant
> > duplication of code from core reload.  This doesn't seem worthwhile
> > at this point ...
> 
> Thinking about it a bit more after our conversation, we do have the
> movw / movt instructions for armv7-a - why push this to the constant
> pool if we can rematerialize it ?  Finding a way to use them rather
> than pushing things out to the constant pool might be an interesting
> experiment for later ..

Reload common code will already choose whatever the best option is
for reloading a constant, according to target hooks (legitimate_constant_p
and preferred_reload_class); it doesn't always push to the constant pool.
However, even on ARM there are currently certain cases where pushing to
the pool is necessary (floating point; some constants involving symbols).

The problem is in this specific case where in an early pass, the back-end
LEGITIMIZE_RELOAD_ADDRESS has already handled an address, and in a later
pass, one of the registers involved turns out to need a reload from a
constant.  In this case, reload common code does not get involved any
more, it expects the back-end to completely handle it.

Now, as I said, the back-end *could* do this, but this would involve
basically duplicating the various checks common code does: does this
particular constant have to be pushed to the literal pool; if so, does
the literal pool address require any further reloads, etc.

It seemed to me that this would be a lot of (tricky and hard to test)
code being added to the back-end, for only very limited gain since this
case should be extremely rare.  Thus my patch simply refused to do any
back-end specific optimization for addresses involving registers that
are equivalent to constants.  This still does not mean everything is
pushed to the constant pool; it just means that reload will fall back
to its default handling if that register is spilled (i.e. checking the
target hooks and doing what's required to load the constant).

> Could you let me know what configuration this was tested with ( i.e.
> the arch. flags ? ) and make sure this also ran through a run with the
> v7-a / softfp /neon configurations ?

I've bootstrapped with the following config options:

  $ ../gcc-head/configure --enable-languages=c,c++,fortran
    --with-arch=armv7-a --with-float=softfp --with-fpu=vfpv3-d16
    --with-mode=thumb --prefix=/home/uweigand/gcc-head-install

(Which I understand are the Ubuntu system compiler settings.)

Is this sufficient, or should I test any other set of options as well?

> Regarding the testcase - please add an -mfloat-abi=soft to the
> testcase so that it tests the specific case that failed in case the
> default configuration was with softfp.

Just to clarify: in the presence of the other options that are already
in dg-options, the test case now fails (with the unpatched compiler)
for *any* setting of -mfloat-abi (hard, soft, or softfp).  Do you still
want me to add a specific setting to the test case?

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com