This is the mail archive of the gcc-patches@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]

fix mem+32760 ICE on powerpc - provide/use predicates weaker than mode_dependent_address_p


Hello,

This is a followup submission for a change first proposed at
http://gcc.gnu.org/ml/gcc-patches/2007-06/msg00767.html

The exact same misbehavior is still observable on mainline, exposed by a
slightly adjusted version of the original testcase (part of the patch):

  bld/gcc $ gcc -B./ -c -DSTACK_OFFSET_BUMP=32700 -O1 offsetmem.c

  offsetmem.c:74:1: error: insn does not satisfy its constraints:
  (insn 203 55 205 2 (set (mem/c:DI (plus:SI (reg/f:SI 1 1)
                (const_int 32760 [0x7ff8])) [0 %sfp+32760 S8 A64])
        (reg:DI 9 9)) y.c:25 317 {*movdi_internal32}
     (nil))

The core issue is the same: a number of places in the compiler use the
mode_dependent_address_p predicate to actually check for weaker necesssary
conditions. Typically, a few places need to check that a MEM access remains
valid when just narrowing the mode, while mode_dependent_address_p tells if
any mode change is valid.

While this is of course generally safe, this has been causing endless troubles
to the powerpc back-end which has apparently unique particularities related to
altivec modes.

This has been worked-around a few times (multiple PRs), ending up with
unpleasant comments such as the ??? piece in this function's head comment:

   /* Go to LABEL if ADDR (a legitimate address expression)
      has an effect that depends on the machine mode it is used for.

      On the RS/6000 this is true of all integral offsets (since AltiVec
      and VSX 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.  */


The attached patch is a proposal to fix this, slightly generalized compared
to the original one. The general idea is to allow for weaker predicates at
the places where we need them.

This is achieved by the introduction of a TARGET_MAY_NARROW_ACCESS target
hook, which defaults to !mode_dependent_address_p and is redefined for
powerpc. The patch uses this hook directly instead of the former predicate in
a couple of places where this was the intent already, as well as new
"valid_access_mode_change_p" function to direct to one or the or the other
depending on provided original and destination modes.

This provides a better match for actual internal needs, allows to get rid of
the powerpc back-end twists (no need to lie in mode_dependent_address_p any
more) and cures the observed internal compiler error.

The patch was bootstrapped and regression testsed for mainline on both x86_64
and powerpc-linux. We (AdaCore) also have been using a gcc-4.5 minor variation
of it without problem for a couple of years on powerpc-aix.

OK to commit ?

Thanks in advance,

With Kind Regards,

Olivier

--

2012-04-26  Olivier Hainque  <hainque@adacore.com>

	* target.def (TARGET_MAY_NARROW_ACCESS_TO): New hook.
	* doc/tm.texi[.in] (TARGET_MAY_NARROW_ACCESS_TO): Document.
	* targhooks.c (default_may_narrow_access_to): Default implementation.
	* targhooks.h (default_may_narrow_access_to): Declare.
	* config/rs6000/rs6000.c (rs6000_may_narrow_access_to): Specific
	implementation.
	(rs6000_mode_dependent_address): Stop lying for + const_int.
	(rs6000_offsettable_memref_p): Adjust comments accordingly.
	* expr.c (convert_move): Use may_narrow_access_to instead of
	mode_dependent_address_p where appropriate.
	* recog.c (offsettable_address_addr_space_p): Likewise.
	(valid_access_mode_change_p): New function.
	* recog.h (valid_access_mode_change_p): Declare.
	* simplify-rtx.c (simplify_subreg): Use it instead of
	mode_dependent_address_p where appropriate.

        testsuite/
        * gcc.dg/offsetmem.c: New test.

Attachment: narrow.dif
Description: video/dv


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