[LRA] Fix ICE for paradoxical subregs on strict-alignment platforms

[Vladimir N Makarov]

On 12/13/2016 05:05 PM, Eric Botcazou wrote:
> Hi,
>
> the Ada runtime library doesn't build on SPARC 32-bit with LRA because of an
> ICE on a couple of files:
>
> eric@polaris:~/build/gcc/sparc-sun-solaris2.10> gcc/gnat1 -I gcc/ada/rts -
> quiet -dumpbase g-debpoo.adb -auxbase g-debpoo -O2 -gnatpg -fPIC -mlra -
> mcpu=v9 g-debpoo.adb -o g-debpoo.s
> +===========================GNAT BUG DETECTED==============================+
> | 7.0.0 20161213 (experimental) [trunk revision 243595] (sparc-sun-
> solaris2.10) GCC error:|
> | in lra_set_insn_recog_data, at lra.c:965                                 |
> | Error detected around g-debpoo.adb:1896:8
>
> The problem is that curr_insn_transform attempts to reload subregs before
> reloading addresses:
>
>    if (! check_only_p)
>      /* Make equivalence substitution and memory subreg elimination
>         before address processing because an address legitimacy can
>         depend on memory mode.  */
>
> Now, on strict-alignment platforms, simplify_operand_subreg has a special
> provision for paradoxical subregs of memory references:
>
> 	 /* If we change the address for a paradoxical subreg of memory, the
> 	    address might violate the necessary alignment or the access might
> 	    be slow.  So take this into consideration.  We need not worry
> 	    about accesses beyond allocated memory for paradoxical memory
> 	    subregs as we don't substitute such equiv memory (see processing
> 	    equivalences in function lra_constraints) and because for spilled
> 	    pseudos we allocate stack memory enough for the biggest
> 	    corresponding paradoxical subreg.  */
> 	  if (!(MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (mode)
> 		&& SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (reg)))
> 	      || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> 		  && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg))))
> 	    return true;
>
> That is to say, if the adjusted memory reference is not sufficient aligned,
> the routine will reload the original memory reference instead of the adjusted
> one on strict-alignment platforms.  But there is a hitch: if the address of
> this original memory reference is invalid, the routine will nevertheless
> reload the entire memory reference, leading to the aforementioned ICE.
>
> The proposed fix is to reload the address, if it is invalid, just before
> reloading the memory reference in simplify_operand_subreg; this yields the
> correct sequence of reloads for the case I investigated.  The patch also
> contains formatting fixes and adds a 'return' for the sake of clarity.
>
> Tested on x86-64/Linux and SPARC/Solaris with LRA, OK for the mainline?
OK.  Thanks for working on this issue, Eric.

The subreg reload code is very sensitive to changes and actually had a 
lot of changes.  So we should be aware that the patch might create new 
PRs.  But I believe the probability of this is tiny.

>
> 2016-12-13  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* lra-constraints.c (process_address): Add forward declaration.
> 	(simplify_operand_subreg): In the MEM case, if the adjusted memory
> 	reference is not sufficient aligned and the address was invalid,
> 	reload the address before reloading the original memory reference.
> 	Fix long lines and add a final return for the sake of clarity.
>