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]

Re: [PATCH] Promoted variable SUBREG optimizations


Its not lightly that I choose to argue with Richard Henderson and
Richard Kenner on SUBREG_PROMOTED_VAR_P.  They've both been personal
heroes since my university days reading the ChangeLogs for v1.37.
However, on this patch I think they're both wrong.


To address RTH's first point:
> (define_insn "*addsi_internal"
>   [(set (match_operand:SI 0 "register_operand" "=r,r,r,r")
>         (plus:SI (match_operand:SI 1 "reg_or_0_operand" "%rJ,rJ,rJ,rJ")
>                  (match_operand:SI 2 "add_operand" "rI,O,K,L")))]
>   ""
>   "@
>    addl %r1,%2,%0
>    subl %r1,%n2,%0
>    lda %0,%2(%r1)
>    ldah %0,%h2(%r1)")

There appears to be nothing in this pattern specific to the
SUBREG_PROMOTED_VAR_P and SUBREG_PROMOTED_UNSIGNED_P flags.  Indeed,
at a guess its just as probably unsafe to match a generic SUBREG (for
example as generated by gen_lowpart) against the operands of the
above pattern.  When used as an rvalue, a SUBREG shouldn't modify/clobber
the inner register.  [I still don't understand the above argument,
so I may still be missing something].

The argument that SUBREG_PROMOTED_VAR_P is only relevant during RTL
expansion doesn't match up with the numerous uses of this macro in
combine.c.  Some of these uses go back to the original 1.1 CVS check-in
in 1997.  If the semantics of SUBREG_PROMOTED_VAR_P weren't preserved
by the RTL optimizers, we'd be seeing problems in nonzero_bits1,
num_sign_bit_copies1, record_promoted_value and check_promoted_subreg.

It also shows very "one dimensional" thinking.  If during RTL expansion,
we emit RTL at the beginning of the function assuming that pseudo X is
sign and/or zero extended and then again at the end of the function
assuming that pseudo X is sign and/or zero extended, we're relying on
all of the later RTL passes preserving that invariant.  If CSE or combine
place a different value in X, the the RTL expander's assumptions will be
just as broken, and the usage of X at the end of the function will be
invalid.


Finally the most convincing argument that this is safe is the experimental
evidence.  The patch below has now been tested without regressions on
alphaev67-dec-osf5.1 (where it fixes the failure of uninit-A.c),
i686-pc-linux-gnu, powerpc-ibm-aix5.2.0.0, ia64-unknown-linux-gnu and
hppa2.0w-hp-hpux11.00, all without new failures.  I'm also still running
bootstraps on mips-sgi-irix6.5 and sparc-sun-solaris2.8 but I won't know
the results for a while :>.  I'm seeing issues with ld-new not being
placed in the gcc subdirectory with uberbaum builds, or I'd test some of
the simulators as well.

I think the burden of proof that this is unsafe is to find a case where
subreg marked as SUBREG_PROMOTED_VAR_P surrounds a pseudo that is not
correctly extended.  Arguments that its possible for backends to mishandle
all SUBREGs just SCO the issue :>  That should be easy, it only takes 1
counter example to contradict the 5 bootstraps and 150K regression tests
above :>.

As a compromise, I'm prepared to concede the simplify-rtx.c hunks below.
It's the dojump.c changes that fix the failure of uninit-A.c on alpha,
and this part only makes use of SUBREG_PROMOTED_VAR_P during RTL
expansion.  However, the GCC source code and all empirical evidence
seems to indicate that the simplify_unary_operation change works in
practice.  I'd rather fix problems than live in fear.

Ok for mainline?


2003-08-09  Roger Sayle  <roger@eyesopen.com>

	* dojump.c (do_jump): If the expression being compared against
	zero, is the subreg of a promoted variable, perform the comparison
	in the promoted mode.
	* simplify-rtx.c (simplify_unary_operation): Optimize sign and
	zero-extensions of subregs of promoted variables where the
	extension is identical to that used to promote the variable.


Index: dojump.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/dojump.c,v
retrieving revision 1.5
diff -c -3 -p -r1.5 dojump.c
*** dojump.c	22 Jul 2003 23:15:26 -0000	1.5
--- dojump.c	9 Aug 2003 03:25:11 -0000
*************** do_jump (tree exp, rtx if_false_label, r
*** 585,591 ****
  	{
  	  /* The RTL optimizers prefer comparisons against pseudos.  */
  	  if (GET_CODE (temp) == SUBREG)
! 	    temp = copy_to_reg (temp);
  	  do_compare_rtx_and_jump (temp, CONST0_RTX (GET_MODE (temp)),
  				   NE, TREE_UNSIGNED (TREE_TYPE (exp)),
  				   GET_MODE (temp), NULL_RTX,
--- 585,598 ----
  	{
  	  /* The RTL optimizers prefer comparisons against pseudos.  */
  	  if (GET_CODE (temp) == SUBREG)
! 	    {
! 	      /* Compare promoted variables in their promoted mode.  */
! 	      if (SUBREG_PROMOTED_VAR_P (temp)
! 		  && GET_CODE (XEXP (temp, 0)) == REG)
! 		temp = XEXP (temp, 0);
! 	      else
! 		temp = copy_to_reg (temp);
! 	    }
  	  do_compare_rtx_and_jump (temp, CONST0_RTX (GET_MODE (temp)),
  				   NE, TREE_UNSIGNED (TREE_TYPE (exp)),
  				   GET_MODE (temp), NULL_RTX,
Index: simplify-rtx.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
retrieving revision 1.151
diff -c -3 -p -r1.151 simplify-rtx.c
*** simplify-rtx.c	19 Jul 2003 14:47:13 -0000	1.151
--- simplify-rtx.c	9 Aug 2003 03:25:14 -0000
*************** simplify_unary_operation (enum rtx_code
*** 821,826 ****
--- 821,835 ----
  	      && GET_CODE (XEXP (XEXP (op, 0), 1)) == LABEL_REF)
  	    return XEXP (op, 0);

+ 	  /* Check for a sign extension of a subreg of a promoted
+ 	     variable, where the promotion is sign-extended, and the
+ 	     target mode is the same as the variable's promotion.  */
+ 	  if (GET_CODE (op) == SUBREG
+ 	      && SUBREG_PROMOTED_VAR_P (op)
+ 	      && ! SUBREG_PROMOTED_UNSIGNED_P (op)
+ 	      && GET_MODE (XEXP (op, 0)) == mode)
+ 	    return XEXP (op, 0);
+
  #if defined(POINTERS_EXTEND_UNSIGNED) && !defined(HAVE_ptr_extend)
  	  if (! POINTERS_EXTEND_UNSIGNED
  	      && mode == Pmode && GET_MODE (op) == ptr_mode
*************** simplify_unary_operation (enum rtx_code
*** 833,840 ****
  #endif
  	  break;

- #if defined(POINTERS_EXTEND_UNSIGNED) && !defined(HAVE_ptr_extend)
  	case ZERO_EXTEND:
  	  if (POINTERS_EXTEND_UNSIGNED > 0
  	      && mode == Pmode && GET_MODE (op) == ptr_mode
  	      && (CONSTANT_P (op)
--- 842,858 ----
  #endif
  	  break;

  	case ZERO_EXTEND:
+ 	  /* Check for a zero extension of a subreg of a promoted
+ 	     variable, where the promotion is zero-extended, and the
+ 	     target mode is the same as the variable's promotion.  */
+ 	  if (GET_CODE (op) == SUBREG
+ 	      && SUBREG_PROMOTED_VAR_P (op)
+ 	      && SUBREG_PROMOTED_UNSIGNED_P (op)
+ 	      && GET_MODE (XEXP (op, 0)) == mode)
+ 	    return XEXP (op, 0);
+
+ #if defined(POINTERS_EXTEND_UNSIGNED) && !defined(HAVE_ptr_extend)
  	  if (POINTERS_EXTEND_UNSIGNED > 0
  	      && mode == Pmode && GET_MODE (op) == ptr_mode
  	      && (CONSTANT_P (op)
*************** simplify_unary_operation (enum rtx_code
*** 843,850 ****
  		      && REG_POINTER (SUBREG_REG (op))
  		      && GET_MODE (SUBREG_REG (op)) == Pmode)))
  	    return convert_memory_address (Pmode, op);
- 	  break;
  #endif

  	default:
  	  break;
--- 861,868 ----
  		      && REG_POINTER (SUBREG_REG (op))
  		      && GET_MODE (SUBREG_REG (op)) == Pmode)))
  	    return convert_memory_address (Pmode, op);
  #endif
+ 	  break;

  	default:
  	  break;

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]