Re: [patch, ARM] PR48250, rehaul arm_legitimize_reload_address()

[Richard Earnshaw <rearnsha@arm.com>]

On Fri, 2011-04-08 at 15:52 +0800, Chung-Lin Tang wrote:
> Hi Richard,
> here's a new patch, with some quite significant changes to
> arm_legitimize_reload_address(). This time fully utilizing the
> sign-magnitude offset macro you gave in the last thread, and adapting
> your description into embedded comments.
> 
> The reload_outdf pattern caused some regressions in a first round of
> testing. With the new reload address legitimization, which creates a RTX
> like (mem (plus (plus reg #high) #low)), the current
> SECONDARY_RELOAD_OUTPUT_CLASS macro returns 'GENERAL_REGS' under VFP,
> which triggers the execution of reload_outdf, but now creating
> unexpected (set (reg1) (plus (plus reg #high) #low)) insns.
> 
> The historical reasons for the reload_outdf pattern are not entirely
> clear, as we discussed off list. In any case, we should aim to update
> the ARM secondary reload stuff to use the newer framework. I have
> disabled the pattern under ARM mode, and cross-tested the entire patch
> under v7-A/v5TE VFP, v7-A NEON, and v4T soft-float configs, all clear of
> regressions.
> 
> reload_outdf is disabled by changing the pattern condition from
> TARGET_32BIT to TARGET_THUMB2 (disabling TARGET_ARM only). This is
> temporary, Thumb-2 probably needs a few more bits of simultaneous
> updates to work, so I'm leaving that for a later patch.
> 
> I've listed your name together as significant parts are from you. Is it
> ready for trunk?
> 
> Thanks,
> Chung-Lin
> 
> 2011-04-08  Chung-Lin Tang  <cltang@codesourcery.com>
> 	    Richard Earnshaw  <rearnsha@arm.com>
> 
> 	PR target/48250
> 	* config/arm/arm.c (arm_legitimize_reload_address): Update cases
> 	to use sign-magnitude offsets. Reject unsupported unaligned
> 	cases. Add detailed description in comments.
> 	* config/arm/arm.md (reload_outdf): Disable for ARM mode; change
> 	condition from TARGET_32BIT to TARGET_ARM.

This is OK, thanks for working on it.

Nothing to do with this patch specifically, but I'll note here for
completeness that LEGITIMIZE_RELOAD_ADDRESS is missing some fundamental
information which makes generating optimal code here impossible: there's
no information available about what the address is going to be used for.
Given that the set of legitimate addresses depends on this is, IMO, a
fundamental problem.

R.