diff mbox

[SH] PR 51244 - Add CANONICALIZE_COMPARISON macro

Message ID 1346659967.2200.68.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo Sept. 3, 2012, 8:12 a.m. UTC 
Hello,

This adds implementation of the CANONICALIZE_COMPARISON macro to the SH
target.  So far, it doesn't seem to have an impact on the generated
code, but it might be useful to have it in place in the future.
Moreover, it changes the behavior of the cbranchsi4 expander, which was
checking for TARGET_CBRANCHDI4, which seems a bit odd to do.
TARGET_CBRANCHDI4 is disabled for -Os (another story) and thus would
affect the behavior of the cbranchsi4 expander.  I've briefly checked
CSiBE result-size with this change and there are only changes in a few
files in the range of -20...+4.
Tested on rev 190865 with
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

and no new failures.
OK?

Cheers,
Oleg

ChangeLog:

	PR target/51244
	* config/sh/sh.c (prepare_cbranch_operands): Pull out 
	comparison canonicalization code into...
	* (sh_canonicalize_comparison): This new function.
	* config/sh/sh-protos.h: Declare it.
	* config/sh/sh.h: Use it in new macro CANONICALIZE_COMPARISON.
	* config/sh/sh.md (cbranchsi4): Remove TARGET_CBRANCHDI4 check 
	and always invoke expand_cbranchsi4.

Comments

Kaz Kojima Sept. 3, 2012, 9:18 a.m. UTC | #1 
Oleg Endo <oleg.endo@t-online.de> wrote:
> --- gcc/config/sh/sh.c	(revision 190840)
> +++ gcc/config/sh/sh.c	(working copy)
> @@ -21,6 +21,12 @@
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>  
> +/* FIXME: This is a temporary hack, so that we can include <algorithm>
> +   below.  <algorithm> will try to include <cstdlib> which will reference
> +   malloc & co, which are poisoned by "system.h".  The proper solution is
> +   to include <cstdlib> in "system.h" instead of <stdlib.h>.  */
> +#include <cstdlib>
> +
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
[snip]
> @@ -1791,65 +1798,124 @@
>      }
>  }
>  
> -enum rtx_code
> -prepare_cbranch_operands (rtx *operands, enum machine_mode mode,
> -			  enum rtx_code comparison)
> +// Implement the CANONICALIZE_COMPARISON macro for the combine pass.
> +// This function is also re-used to canonicalize comparisons in cbranch
> +// pattern expanders.
> +void
> +sh_canonicalize_comparison (enum rtx_code& cmp, rtx& op0, rtx& op1,
> +			    enum machine_mode mode)
>  {
> -  rtx op1;
> -  rtx scratch = NULL_RTX;
> +  // When invoked from within the combine pass the mode is not specified,
> +  // so try to get it from one of the operands.
> +  if (mode == VOIDmode)
> +    mode = GET_MODE (op0);

I'm not sure that the mixture of C and C++ style long comments
in one .c file is OK with the current gcc coding style.  Could
you point me to a reference for that?


Regards,
	kaz
Oleg Endo Sept. 3, 2012, 9:42 a.m. UTC | #2 
On 3 Sep 2012, at 11:18, Kaz Kojima <kkojima@rr.iij4u.or.jp> wrote:

> Oleg Endo <oleg.endo@t-online.de> wrote:
>> --- gcc/config/sh/sh.c    (revision 190840)
>> +++ gcc/config/sh/sh.c    (working copy)
>> @@ -21,6 +21,12 @@
>> along with GCC; see the file COPYING3.  If not see
>> <http://www.gnu.org/licenses/>.  */
>>
>> +/* FIXME: This is a temporary hack, so that we can include  
>> <algorithm>
>> +   below.  <algorithm> will try to include <cstdlib> which will  
>> reference
>> +   malloc & co, which are poisoned by "system.h".  The proper  
>> solution is
>> +   to include <cstdlib> in "system.h" instead of <stdlib.h>.  */
>> +#include <cstdlib>
>> +
>> #include "config.h"
>> #include "system.h"
>> #include "coretypes.h"
> [snip]
>> @@ -1791,65 +1798,124 @@
>>     }
>> }
>>
>> -enum rtx_code
>> -prepare_cbranch_operands (rtx *operands, enum machine_mode mode,
>> -              enum rtx_code comparison)
>> +// Implement the CANONICALIZE_COMPARISON macro for the combine pass.
>> +// This function is also re-used to canonicalize comparisons in  
>> cbranch
>> +// pattern expanders.
>> +void
>> +sh_canonicalize_comparison (enum rtx_code& cmp, rtx& op0, rtx& op1,
>> +                enum machine_mode mode)
>> {
>> -  rtx op1;
>> -  rtx scratch = NULL_RTX;
>> +  // When invoked from within the combine pass the mode is not  
>> specified,
>> +  // so try to get it from one of the operands.
>> +  if (mode == VOIDmode)
>> +    mode = GET_MODE (op0);
>
> I'm not sure that the mixture of C and C++ style long comments
> in one .c file is OK with the current gcc coding style.  Could
> you point me to a reference for that?

Sorry, I can't.  At least The C++ conventions wiki page doesn't  
mention this.  Maybe somebody else can comment on this, please?

In any case, I have no problem with changing the multi line comments  
to /* ... */.  Just let me know.

Cheers,
Oleg
Kaz Kojima Sept. 3, 2012, 10:37 a.m. UTC | #3 
Oleg Endo <oleg.endo@t-online.de> wrote:
> In any case, I have no problem with changing the multi line comments  
> to /* ... */.  Just let me know.

Other than that, the patch is OK.

Regards,
	kaz
diff mbox

Patch

Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	(revision 190840)
+++ gcc/config/sh/sh.md	(working copy)
@@ -881,10 +881,9 @@ 
   if (TARGET_SHMEDIA)
     emit_jump_insn (gen_cbranchint4_media (operands[0], operands[1],
 					   operands[2], operands[3]));
-  else if (TARGET_CBRANCHDI4)
-    expand_cbranchsi4 (operands, LAST_AND_UNUSED_RTX_CODE, -1);
   else
-    sh_emit_compare_and_branch (operands, SImode);
+    expand_cbranchsi4 (operands, LAST_AND_UNUSED_RTX_CODE, -1);
+
   DONE;
 })
 
Index: gcc/config/sh/sh-protos.h
===================================================================
--- gcc/config/sh/sh-protos.h	(revision 190840)
+++ gcc/config/sh/sh-protos.h	(working copy)
@@ -106,6 +106,9 @@ 
 extern rtx sh_gen_truncate (enum machine_mode, rtx, int);
 extern bool sh_vector_mode_supported_p (enum machine_mode);
 extern bool sh_cfun_trap_exit_p (void);
+extern void sh_canonicalize_comparison (enum rtx_code&, rtx&, rtx&,
+					enum machine_mode mode = VOIDmode);
+
 #endif /* RTX_CODE */
 
 extern const char *output_jump_label_table (void);
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 190840)
+++ gcc/config/sh/sh.c	(working copy)
@@ -21,6 +21,12 @@ 
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
+/* FIXME: This is a temporary hack, so that we can include <algorithm>
+   below.  <algorithm> will try to include <cstdlib> which will reference
+   malloc & co, which are poisoned by "system.h".  The proper solution is
+   to include <cstdlib> in "system.h" instead of <stdlib.h>.  */
+#include <cstdlib>
+
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
@@ -56,6 +62,7 @@ 
 #include "tm-constrs.h"
 #include "opts.h"
 
+#include <algorithm>
 
 int code_for_indirect_jump_scratch = CODE_FOR_indirect_jump_scratch;
 
@@ -1791,65 +1798,124 @@ 
     }
 }
 
-enum rtx_code
-prepare_cbranch_operands (rtx *operands, enum machine_mode mode,
-			  enum rtx_code comparison)
+// Implement the CANONICALIZE_COMPARISON macro for the combine pass.
+// This function is also re-used to canonicalize comparisons in cbranch
+// pattern expanders.
+void
+sh_canonicalize_comparison (enum rtx_code& cmp, rtx& op0, rtx& op1,
+			    enum machine_mode mode)
 {
-  rtx op1;
-  rtx scratch = NULL_RTX;
+  // When invoked from within the combine pass the mode is not specified,
+  // so try to get it from one of the operands.
+  if (mode == VOIDmode)
+    mode = GET_MODE (op0);
+  if (mode == VOIDmode)
+    mode = GET_MODE (op1);
 
-  if (comparison == LAST_AND_UNUSED_RTX_CODE)
-    comparison = GET_CODE (operands[0]);
-  else
-    scratch = operands[4];
-  if (CONST_INT_P (operands[1])
-      && !CONST_INT_P (operands[2]))
+  // We need to have a mode to do something useful here.
+  if (mode == VOIDmode)
+    return;
+
+  // Currently, we don't deal with floats here.
+  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
+    return;
+
+  // Make sure that the constant operand is the second operand.
+  if (CONST_INT_P (op0) && !CONST_INT_P (op1))
     {
-      rtx tmp = operands[1];
+      std::swap (op0, op1);
+      cmp = swap_condition (cmp);
+    }
 
-      operands[1] = operands[2];
-      operands[2] = tmp;
-      comparison = swap_condition (comparison);
-    }
-  if (CONST_INT_P (operands[2]))
+  if (CONST_INT_P (op1))
     {
-      HOST_WIDE_INT val = INTVAL (operands[2]);
-      if ((val == -1 || val == -0x81)
-	  && (comparison == GT || comparison == LE))
+      // Try to adjust the constant operand in such a way that available
+      // comparison insns can be utilized better and the constant can be
+      // loaded with a 'mov #imm,Rm' insn.  This avoids a load from the
+      // constant pool.
+      const HOST_WIDE_INT val = INTVAL (op1);
+
+      // x > -1		  --> x >= 0
+      // x > 0xFFFFFF7F	  --> x >= 0xFFFFFF80
+      // x <= -1	  --> x < 0
+      // x <= 0xFFFFFF7F  --> x < 0xFFFFFF80
+      if ((val == -1 || val == -0x81) && (cmp == GT || cmp == LE))
 	{
-	  comparison = (comparison == GT) ? GE : LT;
-	  operands[2] = gen_int_mode (val + 1, mode);
+	  cmp = cmp == GT ? GE : LT;
+	  op1 = gen_int_mode (val + 1, mode);
+        }
+
+      // x >= 1     --> x > 0
+      // x >= 0x80  --> x > 0x7F
+      // x < 1      --> x <= 0
+      // x < 0x80   --> x <= 0x7F
+      else if ((val == 1 || val == 0x80) && (cmp == GE || cmp == LT))
+	{
+	  cmp = cmp == GE ? GT : LE;
+	  op1 = gen_int_mode (val - 1, mode);
 	}
-      else if ((val == 1 || val == 0x80)
-	       && (comparison == GE || comparison == LT))
+
+      // unsigned x >= 1  --> x != 0
+      // unsigned x < 1   --> x == 0
+      else if (val == 1 && (cmp == GEU || cmp == LTU))
 	{
-	  comparison = (comparison == GE) ? GT : LE;
-	  operands[2] = gen_int_mode (val - 1, mode);
+	  cmp = cmp == GEU ? NE : EQ;
+	  op1 = CONST0_RTX (mode);
 	}
-      else if (val == 1 && (comparison == GEU || comparison == LTU))
+
+      // unsigned x >= 0x80  --> unsigned x > 0x7F
+      // unsigned x < 0x80   --> unsigned x < 0x7F
+      else if (val == 0x80 && (cmp == GEU || cmp == LTU))
 	{
-	  comparison = (comparison == GEU) ? NE : EQ;
-	  operands[2] = CONST0_RTX (mode);
+	  cmp = cmp == GEU ? GTU : LEU;
+	  op1 = gen_int_mode (val - 1, mode);
 	}
-      else if (val == 0x80 && (comparison == GEU || comparison == LTU))
+
+      // unsigned x > 0   --> x != 0
+      // unsigned x <= 0  --> x == 0
+      else if (val == 0 && (cmp == GTU || cmp == LEU))
+	cmp = cmp == GTU ? NE : EQ;
+
+      // unsigned x > 0x7FFFFFFF   --> signed x < 0
+      // unsigned x <= 0x7FFFFFFF  --> signed x >= 0
+      else if (mode == SImode && (cmp == GTU || cmp == LEU)
+	       && val == 0x7FFFFFFF)
 	{
-	  comparison = (comparison == GEU) ? GTU : LEU;
-	  operands[2] = gen_int_mode (val - 1, mode);
+	  cmp = cmp == GTU ? LT : GE;
+	  op1 = const0_rtx;
 	}
-      else if (val == 0 && (comparison == GTU || comparison == LEU))
-	comparison = (comparison == GTU) ? NE : EQ;
-      else if (mode == SImode
-	       && ((val == 0x7fffffff
-		    && (comparison == GTU || comparison == LEU))
-		   || ((unsigned HOST_WIDE_INT) val
-			== (unsigned HOST_WIDE_INT) 0x7fffffff + 1
-		       && (comparison == GEU || comparison == LTU))))
+
+      // unsigned x >= 0x80000000  --> signed x < 0
+      // unsigned x < 0x80000000   --> signed x >= 0
+      else if (mode == SImode && (cmp == GEU || cmp == LTU)
+	       && (unsigned HOST_WIDE_INT)val
+		   == ((unsigned HOST_WIDE_INT)0x7FFFFFFF + 1))
 	{
-	  comparison = (comparison == GTU || comparison == GEU) ? LT : GE;
-	  operands[2] = CONST0_RTX (mode);
+	  cmp = cmp == GEU ? LT : GE;
+	  op1 = const0_rtx;
 	}
     }
-  op1 = operands[1];
+}
+
+enum rtx_code
+prepare_cbranch_operands (rtx *operands, enum machine_mode mode,
+			  enum rtx_code comparison)
+{
+  // The scratch reg is only available when this is invoked from within
+  // the cbranchdi4_i splitter, through expand_cbranchdi4.
+  rtx scratch = NULL_RTX;
+
+  if (comparison == LAST_AND_UNUSED_RTX_CODE)
+    comparison = GET_CODE (operands[0]);
+  else
+    scratch = operands[4];
+
+  sh_canonicalize_comparison (comparison, operands[1], operands[2], mode);
+
+  // Notice that this function is also invoked after reload by
+  // the cbranchdi4_i pattern, through expand_cbranchdi4.
+  rtx op1 = operands[1];
+
   if (can_create_pseudo_p ())
     operands[1] = force_reg (mode, op1);
   /* When we are handling DImode comparisons, we want to keep constants so
@@ -1883,8 +1949,6 @@ 
 expand_cbranchsi4 (rtx *operands, enum rtx_code comparison, int probability)
 {
   rtx (*branch_expander) (rtx, rtx) = gen_branch_true;
-  rtx jump;
-
   comparison = prepare_cbranch_operands (operands, SImode, comparison);
   switch (comparison)
     {
@@ -1896,10 +1960,9 @@ 
   emit_insn (gen_rtx_SET (VOIDmode, get_t_reg_rtx (),
                           gen_rtx_fmt_ee (comparison, SImode,
                                           operands[1], operands[2])));
-  jump = emit_jump_insn (branch_expander (operands[3], get_t_reg_rtx ()));
+  rtx jump = emit_jump_insn (branch_expander (operands[3], get_t_reg_rtx ()));
   if (probability >= 0)
     add_reg_note (jump, REG_BR_PROB, GEN_INT (probability));
-
 }
 
 /* ??? How should we distribute probabilities when more than one branch
@@ -1956,8 +2019,7 @@ 
       lsw_taken = EQ;
       if (prob >= 0)
 	{
-	  /* If we had more precision, we'd use rev_prob - (rev_prob >> 32) .
-	   */
+	  // If we had more precision, we'd use rev_prob - (rev_prob >> 32) .
 	  msw_skip_prob = rev_prob;
 	  if (REG_BR_PROB_BASE <= 65535)
 	    lsw_taken_prob = prob ? REG_BR_PROB_BASE : 0;
Index: gcc/config/sh/sh.h
===================================================================
--- gcc/config/sh/sh.h	(revision 190840)
+++ gcc/config/sh/sh.h	(working copy)
@@ -1946,6 +1946,10 @@ 
    leave this zero for correct SH3 code.  */
 #define SHIFT_COUNT_TRUNCATED (! TARGET_SH3 && ! TARGET_SH2A)
 
+/* CANONICALIZE_COMPARISON macro for the combine pass.  */
+#define CANONICALIZE_COMPARISON(CODE, OP0, OP1) \
+  sh_canonicalize_comparison ((CODE), (OP0), (OP1))
+
 /* All integers have the same format so truncation is easy.  */
 /* But SHmedia must sign-extend DImode when truncating to SImode.  */
 #define TRULY_NOOP_TRUNCATION(OUTPREC,INPREC) \