[PATCH] fix ppc ICE on reload spill to mem at boundary offset

[Olivier Hainque <hainque at adacore dot com>]

Hello,

Compiling the simple C testcase sketched below with mainline on
powerpc-aix yields:

<< y.c: In function 'foo':
   y.c:47: error: insn does not satisfy its constraints:
   (insn 111 43 44 2 y.c:24 (set (mem/c:DI (plus:SI (reg/f:SI 1 1)
                (const_int 32760 [0x7ff8])) [0 x13+0 S8 A8])
        (reg:DI 30 30)) 322 {*movdi_internal32} (nil)
    (nil))
   y.c:47: internal compiler error: in reload_cse_simplify_operands,
         at postreload.c:393
>>

This is another manifestation of a recurrent issue with the address in

  (mem/c:DI (plus:SI (reg/f:SI 1 1) (const_int 32760 [0x7ff8])) 

not being recognized as offsettable_address_p in this context on ppc
while it could and presumably should, because of this mode-blind check
in the predicate:

  /* Adjusting an offsettable address involves changing to a narrower mode.
     Make sure that's OK.  */
  if (mode_dependent_address_p (y))
    return 0;

See http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01461.html for
previous instances.

First, a short description of the failure mode, then comments on the
attached patch suggestion.

-------------

Here is a short description of the failure mode. The testcase looks like

   volatile long long V = 1;
   #define STACK_OFFSET_BUMP 32720

   void foo ()
   {
     volatile char bumper [STACK_OFFSET_BUMP];
     register long long x1 = V;
     [...]
     register long long x16 = V;

     bumper [STACK_OFFSET_BUMP-1] = 1;

     V = x1;
     [...]
     V = x16;
   }


We start with

  (insn 43 41 44 2 y.c:24 (set (reg/v:DI 122 [ x13 ])
        (mem/v/c/i:DI (reg/f:SI 135) [0 V+0 S8 A64])) {*movdi_internal32}

Pseudo 122 doesn't get a hard reg, is assigned a stack slot at r1 +
32760 by reload, and here is the insn def:

 ; List r->r after r->"o<>", otherwise reload will try to reload a
 ; non-offsettable address by using r->r which won't make progress.
 (define_insn "*movdi_internal32"
 [(set (match_operand:DI 0 "rs6000_nonimmediate_operand" "=o<>,r,r,*f,*f,m,r")
       (match_operand:DI 1 "input_operand" "r,r,m,f,m,f,IJKnGHF"))]

There is no m-m alternative and the slot address is strictly
legitimate, so ... (from the .greg dump)

   Reloads for insn # 43
   Reload 0: reload_out (DI) = (reg/v:DI 122 [ x13 ])
	   GENERAL_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
	   reload_out_reg: (reg/v:DI 122 [ x13 ])
	   reload_reg_rtx: (reg:DI 30 30)

   (insn 43 41 111 2 y.c:24 (set (reg:DI 30 30)
       (mem/v/c/i:DI (reg/f:SI 9 9 [135]) [0 V+0 S8 A64])) {*movdi_internal32}

   (insn 111 43 44 2 y.c:24 (set (mem/c:DI (plus:SI (reg/f:SI 1 1)
		   (const_int 32760 [0x7ff8])) [0 x13+0 S8 A8])
	   (reg:DI 30 30)) {*movdi_internal32}

insn 111 is an output reload insn which in principle should match the
first constraints alternative (=o<> r), since the mem address indeed
is offsettable in DI mode.

There is a rs6000 specific predicate accurate in this respect. The
generic recog predicate (offsettable_address_p) isn't, however,
because of the aforementioned mode_dependent_address_p check, and we
fail during the postreload pass at

    reload_cse_simplify_operands (rtx insn, rtx testreg)

    /* Figure out which alternative currently matches.  */
    if (! constrain_operands (1))
      fatal_insn_not_found (insn);

because constrain_operands uses the generic predicate.

-------------

The issue revolves around specificities of rs6000_mode_dependent_address
and offsettable_address_p: 

<< On the RS/6000 this is true of all integral offsets (since AltiVec
   modes don't allow them) or is a pre-increment or decrement.

   ??? Except that due to conceptual problems in offsettable_address_p
   we can't really report the problems of integral offsets.  So leave
   this assuming that the adjustable offset must be valid for the
   sub-words of a TFmode operand, which is what we had before.  */
>>

My understanding of the "conceptual problems in offsettable_address_p"
is that mode_dependant_address_p is too strong for this context.
Typically, in the case exposed above, the DI mem would remain valid in
SI or QI modes, and that it would be invalid for an altivec mode is
irrelevant for an "offsettability" check.

This is the idea behind the patch suggested here: introduce a
"may_narrow_access_to (mem, mode)" predicate, weaker than
mode_dependent_address_p for the specific purposes of the generic
offettable_address_p, and adjust rs6000_mode_dependent_address to
do what the comment says it should be doing.

The patch was bootstrapped and reg-tested on powerpc-ibm-aix5.3.0.0
with languages=all,ada.


An alternate way to approach this issue is to consider the history and
current impact of the code for integral offsets (PLUS CONST_INT case)
in rs6000_mode_dependent_address:

AFAICS, this is very old code, long predating Altivec considerations,
initially introduced to perform what the rest of offsettable_address_p
does today, so now useless from this perspective.

Besides, this code only covers a very restricted set of addresses if
I'm not mistaken, so it looks like we already sort of accept that it
may not be doing what it should wrt the altivec considerations in the
vast majority of cases.

Another possible way out of the exposed crash might then be to simply
get rid of the PLUS case in rs6000_mode_dependent_address. Even if
working in practice, this would be violating the current predicate
definition so is not what I chose to evaluate first.

Thanks in advance,

With Kind Regards

Olivier

--

2007-06-12  Olivier Hainque  <hainque@adacore.com>

	gcc/
	* doc/tm.texi (GO_IF_MAY_NARROW_ACCESS_TO): New macro.  Go to
	LABEL if ADDR, a legitimate address expression for MODE, may
	be accessed using a narrower mode for the purpose of an "offsettable"
	reference.
	* recog.c (GO_IF_MAY_NARROW_ACCESS_TO): Default definition.  Win if
	ADDR is mode independent.
	(may_narrow_access_to): New predicate. Wrapper around the new GO_IF
	macro for an address and a mode.
	(offettable_address_p): Use it instead of mode_dependent_address_p.
	* config/rs6000/rs6000.h (GO_IF_MAY_NARROW_ACCESS_TO): Map to ...
	* config/rs6000/rs6000.c (rs6000_may_narrow_access_to): New function.
	Negation of the current rs6000_mode_dependent_address without the
	CONST_INT offsets tweaks.
	(rs6000_mode_dependent_address): Always return true for PLUS CONST_INT
	and adjust head comment accordingly.
	* config/rs6000/rs6000-protos.h (rs6000_may_narrow_access_to): Declare.

	testsuite/
	* gcc.dg/offsets.c: New case.

Attachment: ppc-offsets.dif
Description: Text document