This is the mail archive of the gcc@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: [RFD] Simplifying subregs in LRA




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.


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