This is the mail archive of the
mailing list for the GCC project.
Re: [RFD] Simplifying subregs in LRA
- From: Vladimir Makarov <vmakarov at redhat dot com>
- To: Matthew Fortune <Matthew dot Fortune at imgtec dot com>, "Eric Botcazou (ebotcazou at adacore dot com)" <ebotcazou at adacore dot com>, "Alan Modra (amodra at gmail dot com)" <amodra at gmail dot com>, "wmi at google dot com" <wmi at google dot com>
- Cc: "gcc at gcc dot gnu dot org" <gcc at gcc dot gnu dot org>
- Date: Fri, 3 Feb 2017 13:14:49 -0500
- Subject: Re: [RFD] Simplifying subregs in LRA
- Authentication-results: sourceware.org; auth=none
- References: <6D39441BF12EF246A7ABCE6654B0235380B55AD3@hhmail02.hh.imgtec.org>
On 02/01/2017 06:52 PM, Matthew Fortune wrote:
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.
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.
minated on the next iteration
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 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.
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
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?
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
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
@@ -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)))))
*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.