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 1/2][GCC] Refactor noce_try_store_flag_constants


This patch refactors 'noce_try_store_flag_constants' a bit to make it easier to
reason about.

The function contains two series of conditions, and each branch of the second
series corresponds to one or two branches of the first series. The patch
introduces a new enumeration strategy_t instead and uses it to select the
correct branch. Also, ISTM that the last 'else' branch is unreachable.

Bootstrapped and regtested on x86_64-linux. OK for trunk?

-- 
Regards,
    Mikhail Maltsev

gcc/ChangeLog:

2016-05-23  Mikhail Maltsev  <maltsevm@gmail.com>

        * ifcvt.c (noce_try_store_flag_constants): Refactor.

From 847ba5ac9194273c8b51839cfda86bbc399847f4 Mon Sep 17 00:00:00 2001
From: Mikhail Maltsev <maltsevm@gmail.com>
Date: Tue, 10 May 2016 22:53:26 +0300
Subject: [PATCH 1/2] Refactor noce_try_store_flag_constants


diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 4949965..a9c146b 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1256,13 +1256,14 @@ noce_try_inverse_constants (struct noce_if_info *if_info)
 static int
 noce_try_store_flag_constants (struct noce_if_info *if_info)
 {
-  rtx target;
-  rtx_insn *seq;
-  bool reversep;
-  HOST_WIDE_INT itrue, ifalse, diff, tmp;
-  int normalize;
-  bool can_reverse;
-  machine_mode mode = GET_MODE (if_info->x);;
+  enum strategy_t
+  {
+    ST_ADD_FLAG,
+    ST_SHIFT_FLAG,
+    ST_IOR_FLAG
+  };
+
+  machine_mode mode = GET_MODE (if_info->x);
   rtx common = NULL_RTX;
 
   rtx a = if_info->a;
@@ -1292,11 +1293,11 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
   if (CONST_INT_P (a)
       && CONST_INT_P (b))
     {
-      ifalse = INTVAL (a);
-      itrue = INTVAL (b);
+      HOST_WIDE_INT ifalse = INTVAL (a);
+      HOST_WIDE_INT itrue = INTVAL (b);
       bool subtract_flag_p = false;
 
-      diff = (unsigned HOST_WIDE_INT) itrue - ifalse;
+      HOST_WIDE_INT diff = (unsigned HOST_WIDE_INT) itrue - ifalse;
       /* Make sure we can represent the difference between the two values.  */
       if ((diff > 0)
 	  != ((ifalse < 0) != (itrue < 0) ? ifalse < 0 : ifalse < itrue))
@@ -1304,12 +1305,15 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
 
       diff = trunc_int_for_mode (diff, mode);
 
-      can_reverse = (reversed_comparison_code (if_info->cond, if_info->jump)
-		     != UNKNOWN);
+      bool can_reverse
+	= (reversed_comparison_code (if_info->cond, if_info->jump) != UNKNOWN);
 
-      reversep = false;
+      bool reversep = false;
+      int normalize;
+      strategy_t strategy;
       if (diff == STORE_FLAG_VALUE || diff == -STORE_FLAG_VALUE)
 	{
+	  strategy = ST_ADD_FLAG;
 	  normalize = 0;
 	  /* We could collapse these cases but it is easier to follow the
 	     diff/STORE_FLAG_VALUE combinations when they are listed
@@ -1355,20 +1359,28 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
       else if (ifalse == 0 && exact_log2 (itrue) >= 0
 	       && (STORE_FLAG_VALUE == 1
 		   || if_info->branch_cost >= 2))
-	normalize = 1;
+	{
+	  strategy = ST_SHIFT_FLAG;
+	  normalize = 1;
+	}
       else if (itrue == 0 && exact_log2 (ifalse) >= 0 && can_reverse
 	       && (STORE_FLAG_VALUE == 1 || if_info->branch_cost >= 2))
 	{
+	  strategy = ST_SHIFT_FLAG;
 	  normalize = 1;
 	  reversep = true;
 	}
       else if (itrue == -1
 	       && (STORE_FLAG_VALUE == -1
 		   || if_info->branch_cost >= 2))
-	normalize = -1;
+	{
+	  strategy = ST_IOR_FLAG;
+	  normalize = -1;
+	}
       else if (ifalse == -1 && can_reverse
 	       && (STORE_FLAG_VALUE == -1 || if_info->branch_cost >= 2))
 	{
+	  strategy = ST_IOR_FLAG;
 	  normalize = -1;
 	  reversep = true;
 	}
@@ -1391,59 +1403,56 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
 	  noce_emit_move_insn (common, if_info->x);
 	}
 
-      target = noce_emit_store_flag (if_info, if_info->x, reversep, normalize);
+      rtx target
+	= noce_emit_store_flag (if_info, if_info->x, reversep, normalize);
       if (! target)
 	{
 	  end_sequence ();
 	  return FALSE;
 	}
 
-      /* if (test) x = 3; else x = 4;
-	 =>   x = 3 + (test == 0);  */
-      if (diff == STORE_FLAG_VALUE || diff == -STORE_FLAG_VALUE)
+      if (common && strategy != ST_ADD_FLAG)
+	{
+	  /* Not beneficial when the original A and B are PLUS expressions.  */
+	  end_sequence ();
+	  return false;
+	}
+
+      switch (strategy)
 	{
+	case ST_ADD_FLAG:
+	  /* if (test) x = 3; else x = 4;
+	     =>   x = 3 + (test == 0);  */
+
 	  /* Add the common part now.  This may allow combine to merge this
 	     with the store flag operation earlier into some sort of conditional
 	     increment/decrement if the target allows it.  */
 	  if (common)
-	    target = expand_simple_binop (mode, PLUS,
-					   target, common,
-					   target, 0, OPTAB_WIDEN);
+	    target = expand_simple_binop (mode, PLUS, target, common, target, 0,
+					  OPTAB_WIDEN);
 
 	  /* Always use ifalse here.  It should have been swapped with itrue
 	     when appropriate when reversep is true.  */
 	  target = expand_simple_binop (mode, subtract_flag_p ? MINUS : PLUS,
 					gen_int_mode (ifalse, mode), target,
 					if_info->x, 0, OPTAB_WIDEN);
-	}
-      /* Other cases are not beneficial when the original A and B are PLUS
-	 expressions.  */
-      else if (common)
-	{
-	  end_sequence ();
-	  return FALSE;
-	}
-      /* if (test) x = 8; else x = 0;
-	 =>   x = (test != 0) << 3;  */
-      else if (ifalse == 0 && (tmp = exact_log2 (itrue)) >= 0)
-	{
-	  target = expand_simple_binop (mode, ASHIFT,
-					target, GEN_INT (tmp), if_info->x, 0,
-					OPTAB_WIDEN);
-	}
-
-      /* if (test) x = -1; else x = b;
-	 =>   x = -(test != 0) | b;  */
-      else if (itrue == -1)
-	{
-	  target = expand_simple_binop (mode, IOR,
-					target, gen_int_mode (ifalse, mode),
-					if_info->x, 0, OPTAB_WIDEN);
-	}
-      else
-	{
-	  end_sequence ();
-	  return FALSE;
+	  break;
+	case ST_SHIFT_FLAG:
+	  {
+	    /* if (test) x = 8; else x = 0;
+	       =>   x = (test != 0) << 3;  */
+	    HOST_WIDE_INT tmp = exact_log2 (itrue);
+	    target = expand_simple_binop (mode, ASHIFT, target, GEN_INT (tmp),
+					  if_info->x, 0, OPTAB_WIDEN);
+	    break;
+	  }
+	case ST_IOR_FLAG:
+	  /* if (test) x = -1; else x = b;
+	     =>   x = -(test != 0) | b;  */
+	  target = expand_simple_binop (mode, IOR, target,
+					gen_int_mode (ifalse, mode), if_info->x,
+					0, OPTAB_WIDEN);
+	  break;
 	}
 
       if (! target)
@@ -1455,7 +1464,7 @@ noce_try_store_flag_constants (struct noce_if_info *if_info)
       if (target != if_info->x)
 	noce_emit_move_insn (if_info->x, target);
 
-      seq = end_ifcvt_sequence (if_info);
+      rtx_insn *seq = end_ifcvt_sequence (if_info);
       if (!seq)
 	return FALSE;
 
-- 
2.1.4


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