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]

[PATCH] LOAD_EXTEND_OP only applies to (subreg (mem ...))


This patch is the result of my investigation into high priortity
PR c/2454, which has been discussed on the GCC mailing list.
See http://gcc.gnu.org/ml/gcc/2002-07/msg00007.html

Most of the original issue of PR c/2454 has been fixed by Joseph Myers'
front-end patch, but the PR was never closed because the testcase
continued to fail on a few targets, indicating an additional backend
problem.


I originally came up with this patch over a week ago, but wasn't
sure it was the correct solution until now.  Whilst working on an
possible alternate fix to the problem, I came across the comment
at about line 11,000 of combine.c (in simplify_comparison).  This
page long description of the semantics of SUBREGs is the best
documentation I've come across so far, and has confirmed my belief
that LOAD_EXTEND_OP should not apply to SUB_REGs of REGs.

The bug is that simplify_comparison believes that (subreg (reg ...))
has undefined upper bits, but the two functions nonzero_bits and
num_sign_bit_copies believe (subreg (reg ...)) is extended according
to LOAD_EXTEND_OP.  The fix below is to tighten the extension tests
in these two functions to check "GET_CODE (SUBREG_REG (x)) == MEM".

The following code has been tested on v850-sim using a v850-elf
cross-compiler built from i686-pc-linux-gnu, with no new failures
with "make -k check-gcc" where it fixes the five 20011223-1.c
failures (20011223-1.c is the example submitted with PR c/2454
in GNATS).  Its also been tested with a complete "make bootstrap"
and "make -k check", all languages except Ada and treelang, on
both i686-pc-linux-gnu and hppa2.0w-hp-hpux11.00 with no new
regressions.

IA32 doesn't define WORD_REGISTER_OPERATIONS, HP PA2.0 defines
WORD_REGISTER_OPERATIONS and LOAD_EXTEND_OP to ZERO_EXTEND, and
v850 defines WORD_REGISTER_OPERATIONS and LOAD_EXTEND_OP to be
SIGN_EXTEND.  Hence all major combinations have been tested.


Ok for mainline and to close high priority PR c/2454?


[p.s. I completely agree with Jeff Law that these semantics of
SUBREGs are broken, and should be replaced by explicit zero_extend
or sign_extend RTL some day.  There are numerous places where GCC
can substitute a MEM for a REG, or a REG for a MEM, that would end
up changing the meaning of the RTL.  Fortunately many backends,
including IA32, use explicit zero_extend and sign_extend patterns]


2002-07-09  Roger Sayle  <roger@eyesopen.com>

	PR c/2454
	* combine.c (nonzero_bits): LOAD_EXTEND_OP should only apply
	to SUBREGs of MEMs.  (num_sign_bit_copies): Likewise.


Index: combine.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.304
diff -c -3 -p -r1.304 combine.c
*** combine.c	8 Jul 2002 15:59:53 -0000	1.304
--- combine.c	8 Jul 2002 19:19:31 -0000
*************** nonzero_bits (x, mode)
*** 8344,8355 ****
  #if defined (WORD_REGISTER_OPERATIONS) && defined (LOAD_EXTEND_OP)
  	  /* If this is a typical RISC machine, we only have to worry
  	     about the way loads are extended.  */
! 	  if (LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND
! 	      ? (((nonzero
! 		   & (((unsigned HOST_WIDE_INT) 1
! 		       << (GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (x))) - 1))))
! 		  != 0))
! 	      : LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) != ZERO_EXTEND)
  #endif
  	    {
  	      /* On many CISC machines, accessing an object in a wider mode
--- 8344,8356 ----
  #if defined (WORD_REGISTER_OPERATIONS) && defined (LOAD_EXTEND_OP)
  	  /* If this is a typical RISC machine, we only have to worry
  	     about the way loads are extended.  */
! 	  if ((LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND
! 	       ? (((nonzero
! 		    & (((unsigned HOST_WIDE_INT) 1
! 			<< (GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (x))) - 1))))
! 		   != 0))
! 	       : LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) != ZERO_EXTEND)
! 	      || GET_CODE (SUBREG_REG (x)) != MEM)
  #endif
  	    {
  	      /* On many CISC machines, accessing an object in a wider mode
*************** num_sign_bit_copies (x, mode)
*** 8572,8578 ****

        if ((GET_MODE_SIZE (GET_MODE (x))
  	   > GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
! 	  && LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND)
  	return num_sign_bit_copies (SUBREG_REG (x), mode);
  #endif
  #endif
--- 8573,8580 ----

        if ((GET_MODE_SIZE (GET_MODE (x))
  	   > GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
! 	  && LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND
! 	  && GET_CODE (SUBREG_REG (x)) == MEM)
  	return num_sign_bit_copies (SUBREG_REG (x), mode);
  #endif
  #endif

Roger
--
Roger Sayle,                         E-mail: roger@eyesopen.com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833


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