[RFD] Simplifying subregs in LRA

[Vladimir Makarov]

On 02/01/2017 06:52 PM, Matthew Fortune wrote:
> Hi all,
>
> I've copied you as you have each made some significant change to a function
> in LRA which I guess makes you de-facto experts.
>
> I've spent a while researching the history of simplify_operand_subreg and
> in particular the behaviour for subregs of memory.  For my sake if no-one
> else here is a rundown of its evolution; corrections welcome.
Thanks for doing the research, Matt.
> minated on the next iteration
>
> (r198344 git:ea99c7a)
> A special case for an LRA introduced subreg was added (LRA_SUBREG_P) that
> should always be considered valid.  This I believe is to cope with cases
> where a there are operands required to match but with different modes and,
> presumably, one of the modes is not actually allowed.  Not 100% sure what
> this is though!
As I remember x86 has such tricking x87 fp stack insn definitions with 
matching operands of different modes.  As reload does all RTL changes at 
once at its work end,  it was not a problem.  LRA transforms RTL 
permanently during its work and existing RTL illegal at some point might 
be a problem.
...
> So what are the next steps!
>
> 1) [BUG] Add an exclusion for WORD_REGISTER_OPERATIONS because MIPS is currently
>     broken by the existing code. PR78660
> 2) [BUG] Remove the return false introduced in (r243782 git:856bd6f).
> 3) [CLEANUP] Remove reg_mode argument and replace all uses of reg_mode with
>     innermode.  Rename 'reg' to 'inner' and 'operand' to 'outer' and 'mode' to
>     'outermode'.
> 4) [OPTIMISATION] Change double-reload logic so that it just deals with the
>     special outermode reload without adjusting the subreg.
> 5) [??] Determine if big endian still needs a special case like in reload?
>     Comments anyone?
As Eric I prefer changes which affect minimum targets and minimum cases 
but still fix the bug. Almost any change in this part of LRA required 
some stabilization changes.  It is dangerous to do a big cleanup at this 
development stage of GCC.

Bigger cleanup could be done on stage1 after GCC7 release.  I hope you 
and Eric will do this.  Reload is a good reference point. Historically, 
LRA was originally written for few targets without taking corner cases 
of all other targets.  Trying to implement all reload cases without good 
understanding them or their necessity would have made LRA as a project 
impossible.
> In an attempt to make a minimal change I propose the following as it allows
> WORD_REGISTER_OPERATIONS targets to benefit from the invalid address reloading
> fix. I think the check would be more appropriately placed on the outer-most
> if (MEM_P (reg)) but this would affect the handling of many more subregs which
> seems too dangerous at this point in release.
>
> diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
> index 393cc69..771475a 100644
> --- a/gcc/lra-constraints.c
> +++ b/gcc/lra-constraints.c
> @@ -1512,10 +1512,11 @@ simplify_operand_subreg (int nop, machine_mode reg_mode)
>   	     equivalences in function lra_constraints) and because for spilled
>   	     pseudos we allocate stack memory enough for the biggest
>   	     corresponding paradoxical subreg.  */
> -	  if (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
> -		&& SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
> -	      || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> -		  && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg))))
> +	  if (!WORD_REGISTER_OPERATIONS
> +	      && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
> +		    && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
> +		  || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> +		      && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg)))))
>   	    return true;
>   
>   	  *curr_id->operand_loc[nop] = operand;
>
>
> The change will affect at least arc,mips,rx,sh,sparc though I haven't checked
> which of these default on for LRA just that they can turn on LRA.
>
> I'll post this as a patch with appropriate updates to comments unless anyone
> raises some issues.
>
OK.  Thanks.