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 1/2, combine] Try REG_EQUAL for nonzero_bits


Hi Eric,

I'm taking over Zhenqiang's work on this. Comments and updated patch
below.

> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Eric Botcazou
> > +  rtx reg_equal = insn ? find_reg_equal_equiv_note (insn) : NULL_RTX;
> > +  unsigned HOST_WIDE_INT bits = nonzero_bits (src,
> nonzero_bits_mode);
> 
> Note that "src" has taken the SHORT_IMMEDIATES_SIGN_EXTEND path
> here.
> 
> > +  if (reg_equal)
> > +    {
> > +      unsigned int num = num_sign_bit_copies (XEXP (reg_equal, 0),
> > +					      GET_MODE (x));
> > +      bits &= nonzero_bits (XEXP (reg_equal, 0), nonzero_bits_mode);
> 
> But XEXP (reg_equal, 0) hasn't here.  If we want to treat the datum of
> the
> REG_EQUAL or REG_EQUIV note as equivalent to the SET_SRC (set), and
> I think we
> should (see for example combine.c:9650), there is a problem.
> 
> So I think we should create a new function, something along of:
> 
> /* If MODE has a precision lower than PREC and SRC is a non-negative
> constant
>    that would appear negative in MODE, sign-extend SRC for use in
> nonzero_bits
>    because some machines (maybe most) will actually do the sign-
> extension and
>    this is the conservative approach.
> 
>    ??? For 2.5, try to tighten up the MD files in this regard
>    instead of this kludge.  */
> 
> rtx
> sign_extend_short_imm (rtx src, machine_mode mode, unsigned int
> prec)
> {
>   if (GET_MODE_PRECISION (mode) < prec
> 	&& CONST_INT_P (src)
> 	&& INTVAL (src) > 0
> 	&& val_signbit_known_set_p (mode, INTVAL (src)))
>     src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (mode));
> 
>   return src;
> }
> 
> and calls it from combine.c:1702
> 
> #ifdef SHORT_IMMEDIATES_SIGN_EXTEND
>   src = sign_extend_short_imm (src, GET_MODE (x), BITS_PER_WORD);
> #endif
> 
> and from combine.c:9650
> 
> #ifdef SHORT_IMMEDIATES_SIGN_EXTEND
>   tem = sign_extend_short_imm (tem, GET_MODE (x),
> GET_MODE_PRECISION (mode));
> #endif
> 
> Once this is done, the same thing needs to be applied to XEXP
> (reg_equal, 0)
> before it is sent to nonzero_bits.

I did this as you suggested and decided to split the patch in 2 to make it easier
to review. Part 1 does this reorganization while part 2 concern the REG_EQUAL
matter.

ChangeLog entry for part 1 is as follows:

*** gcc/ChangeLog ***

2015-02-09  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * combine.c (sign_extend_short_imm): New.
        (set_nonzero_bits_and_sign_copies): Use above new function for sign
        extension of src short immediate.
        (reg_nonzero_bits_for_combine): Likewise for tem.

diff --git a/gcc/combine.c b/gcc/combine.c
index ad3bed0..f2b26c2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1640,6 +1640,26 @@ setup_incoming_promotions (rtx_insn *first)
     }
 }
 
+#ifdef SHORT_IMMEDIATES_SIGN_EXTEND
+/* If MODE has a precision lower than PREC and SRC is a non-negative constant
+   that would appear negative in MODE, sign-extend SRC for use in nonzero_bits
+   because some machines (maybe most) will actually do the sign-extension and
+   this is the conservative approach.
+
+   ??? For 2.5, try to tighten up the MD files in this regard instead of this
+   kludge.  */
+
+static rtx
+sign_extend_short_imm (rtx src, machine_mode mode, unsigned int prec)
+{
+  if (GET_MODE_PRECISION (mode) < prec && CONST_INT_P (src)
+      && INTVAL (src) > 0 && val_signbit_known_set_p (mode, INTVAL (src)))
+    src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (mode));
+
+  return src;
+}
+#endif
+
 /* Called via note_stores.  If X is a pseudo that is narrower than
    HOST_BITS_PER_WIDE_INT and is being set, record what bits are known zero.
 
@@ -1719,20 +1739,7 @@ set_nonzero_bits_and_sign_copies (rtx x, const_rtx set, void *data)
 	  rtx src = SET_SRC (set);
 
 #ifdef SHORT_IMMEDIATES_SIGN_EXTEND
-	  /* If X is narrower than a word and SRC is a non-negative
-	     constant that would appear negative in the mode of X,
-	     sign-extend it for use in reg_stat[].nonzero_bits because some
-	     machines (maybe most) will actually do the sign-extension
-	     and this is the conservative approach.
-
-	     ??? For 2.5, try to tighten up the MD files in this regard
-	     instead of this kludge.  */
-
-	  if (GET_MODE_PRECISION (GET_MODE (x)) < BITS_PER_WORD
-	      && CONST_INT_P (src)
-	      && INTVAL (src) > 0
-	      && val_signbit_known_set_p (GET_MODE (x), INTVAL (src)))
-	    src = GEN_INT (INTVAL (src) | ~GET_MODE_MASK (GET_MODE (x)));
+	  src = sign_extend_short_imm (src, GET_MODE (x), BITS_PER_WORD);
 #endif
 
 	  /* Don't call nonzero_bits if it cannot change anything.  */
@@ -9770,20 +9777,8 @@ reg_nonzero_bits_for_combine (const_rtx x, machine_mode mode,
   if (tem)
     {
 #ifdef SHORT_IMMEDIATES_SIGN_EXTEND
-      /* If X is narrower than MODE and TEM is a non-negative
-	 constant that would appear negative in the mode of X,
-	 sign-extend it for use in reg_nonzero_bits because some
-	 machines (maybe most) will actually do the sign-extension
-	 and this is the conservative approach.
-
-	 ??? For 2.5, try to tighten up the MD files in this regard
-	 instead of this kludge.  */
-
-      if (GET_MODE_PRECISION (GET_MODE (x)) < GET_MODE_PRECISION (mode)
-	  && CONST_INT_P (tem)
-	  && INTVAL (tem) > 0
-	  && val_signbit_known_set_p (GET_MODE (x), INTVAL (tem)))
-	tem = GEN_INT (INTVAL (tem) | ~GET_MODE_MASK (GET_MODE (x)));
+      tem = sign_extend_short_imm (tem, GET_MODE (x),
+				   GET_MODE_PRECISION (mode));
 #endif
       return tem;
     }

Is this ok for next stage1?

Best regards,

Thomas




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