diff mbox

[PING^2,2,of,2] RTL expansion for zero sign extension elimination with VRP

Message ID 52CBEC0D.4070706@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Jan. 7, 2014, 11:59 a.m. UTC 
ping ?

I have reorganised the last patch and now handling only
VIEW_CONVERT_EXPR, CONVERT_EXPR and NOP_EXPR. Once it is reviewed and
necessary changes are made, I will address the other cases as a separate
patch (when it reaches that stage).

Thanks,
Kugan

gcc/

+2014-01-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
+
+	* dojump.c (do_compare_and_jump): Generate rtl without
+	zero/sign extension if redundant.
+	* cfgexpand.c (expand_gimple_stmt_1): Likewise.
+	(is_assigned_exp_fit_type) : New function.
+	* cfgexpand.h (is_assigned_exp_fit_type) : Declare.
+

Comments

Kugan Vivekanandarajah Jan. 8, 2014, 7:39 a.m. UTC | #1 
On 07/01/14 23:23, Richard Biener wrote:
> On Tue, 7 Jan 2014, Kugan wrote:

[snip]


> Note that VIEW_CONVERT_EXPR is wrong here.  I think you are
> handling this wrong still.  From a quick look you want to avoid
> the actual promotion for
> 
>   reg_1 = ....
> 
> when reg_1 is promoted and thus the target is (subreg:XX N).
> The RHS has been expanded in XXmode.  Dependent on the value-range
> of reg_1 you want to set N to a paradoxical subreg of the expanded
> result.  You can always do that if the reg is zero-extended
> and else if the MSB is not set for any of the values of reg_1.

Thanks Richard for the explanation. I just want to double confirm I
understand you correctly before I attempt to fix it. So let me try this
for the following example,

for a gimple stmt of the following from:
unsigned short _5;
short int _6;
_6 = (short int)_5;

;; _6 = (short int) _5;
target = (subreg/s/u:HI (reg:SI 110 [ D.4144 ]) 0)
temp = (subreg:HI (reg:SI 118) 0)

So, I must generate the following if it satisfies the other conditions.
(set (reg:SI 110 [ D.4144 ]) (subreg:SI temp ))

Is my understanding correct?

> I don't see how is_assigned_exp_fit_type reflects this in any way.
>


What I tried doing with the patch is:

(insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
        (zero_extend:SI (subreg:HI (reg:SI 118) 0))) c5.c:8 -1
     (nil))

If the values in register (reg:SI 118) fits HI mode (without
overflowing), I assume that it is not necessary to just drop the higher
bits and zero_extend as done above and generate the following instead.

(insn 13 12 0 (set (reg:SI 110 [ D.4144 ])
        (((reg:SI 118) 0))) c5.c:8 -1
     (nil))

is_assigned_exp_fit_type just checks if the range fits (in the above
case, the value in eg:SI 118 fits HI mode) and the checks before
emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp)); checks the
modes match.

Is this wrong  or am I missing the whole point?

> Anyway, the patch should not introduce another if (promoted)
> case but only short-cut the final convert_move call of the existing
> one.
>


Thanks,
Kugan
diff mbox

Patch

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 7a93975..b2e2f90 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -476,6 +476,66 @@  add_scope_conflicts_1 (basic_block bb, bitmap work, bool for_conflict)
     }
 }
 
+
+/* Check gimple assign stmt and see if zero/sign extension is
+   redundant.  i.e.  if an assignment gimple statement has RHS expression
+   value that can fit in LHS type, subreg and extension to fit can be
+   redundant.  Zero/sign extensions in this case can be removed.  */
+
+bool
+is_assigned_exp_fit_type (tree lhs)
+{
+  double_int type_min, type_max;
+  double_int min1, max1;
+  enum tree_code stmt_code;
+  tree rhs1;
+  gimple stmt = SSA_NAME_DEF_STMT (lhs);
+
+  if (gimple_code (stmt) != GIMPLE_ASSIGN)
+    return false;
+
+  /* We remove extension for non-pointer and integral stmts.  */
+  if (!INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+      || POINTER_TYPE_P (TREE_TYPE (lhs)))
+    return false;
+
+  stmt_code = gimple_assign_rhs_code (stmt);
+  rhs1 = gimple_assign_rhs1 (stmt);
+  type_max = tree_to_double_int (TYPE_MAX_VALUE (TREE_TYPE (lhs)));
+  type_min = tree_to_double_int (TYPE_MIN_VALUE (TREE_TYPE (lhs)));
+
+  if (TREE_CODE_CLASS (stmt_code) == tcc_unary)
+    {
+      bool uns = TYPE_UNSIGNED (TREE_TYPE (rhs1));
+      /* Get the value range.  */
+      if (TREE_CODE (rhs1) == INTEGER_CST)
+	{
+	  min1 = tree_to_double_int (rhs1);
+	  max1 = tree_to_double_int (rhs1);
+	}
+      else if (get_range_info (rhs1, &min1, &max1) != VR_RANGE)
+	return false;
+
+      switch (stmt_code)
+	{
+	case VIEW_CONVERT_EXPR:
+	case CONVERT_EXPR:
+	case NOP_EXPR:
+	  /* If rhs value range fits lhs type, zero/sign extension is
+	    redundant.  */
+	  if (max1.cmp (type_max, 0) != 1
+	      && (type_min.cmp (min1, 0)) != 1)
+	    return true;
+	  else
+	    return false;
+	default:
+	  return false;
+	}
+    }
+
+  return false;
+}
+
 /* Generate stack partition conflicts between all partitions that are
    simultaneously live.  */
 
@@ -3247,6 +3307,20 @@  expand_gimple_stmt_1 (gimple stmt)
 
 	    if (temp == target)
 	      ;
+	    /* If the value in SUBREG of temp fits that SUBREG (does not
+	       overflow) and is assigned to target SUBREG of the same mode
+	       without sign conversion, we can skip the SUBREG
+	       and extension.  */
+	    else if (promoted
+		     && is_assigned_exp_fit_type (lhs)
+		     && (GET_CODE (temp) == SUBREG)
+		     && (GET_MODE_PRECISION (GET_MODE (SUBREG_REG (temp)))
+			 >= GET_MODE_PRECISION (GET_MODE (target)))
+		     && (GET_MODE (SUBREG_REG (target))
+			 == GET_MODE (SUBREG_REG (temp))))
+	      {
+		emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
+	      }
 	    else if (promoted)
 	      {
 		int unsignedp = SUBREG_PROMOTED_UNSIGNED_P (target);
diff --git a/gcc/cfgexpand.h b/gcc/cfgexpand.h
index 04517a3..c7d73e8 100644
--- a/gcc/cfgexpand.h
+++ b/gcc/cfgexpand.h
@@ -22,5 +22,6 @@  along with GCC; see the file COPYING3.  If not see
 
 extern tree gimple_assign_rhs_to_tree (gimple);
 extern HOST_WIDE_INT estimated_stack_frame_size (struct cgraph_node *);
+extern bool is_assigned_exp_fit_type (tree lhs);
 
 #endif /* GCC_CFGEXPAND_H */
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 73df6d1..73a4b6b 100644
--- a/gcc/dojump.c
+++ b/gcc/dojump.c
@@ -35,6 +35,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "ggc.h"
 #include "basic-block.h"
 #include "tm_p.h"
+#include "cfgexpand.h"
 
 static bool prefer_and_bit_test (enum machine_mode, int);
 static void do_jump_by_parts_greater (tree, tree, int, rtx, rtx, int);
@@ -1166,6 +1167,62 @@  do_compare_and_jump (tree treeop0, tree treeop1, enum rtx_code signed_code,
 
   type = TREE_TYPE (treeop0);
   mode = TYPE_MODE (type);
+
+  /* Is zero/sign extension redundant.  */
+  bool op0_ext_redundant = false;
+  bool op1_ext_redundant = false;
+
+  /* If promoted and the value in SUBREG of op0 fits (does not overflow),
+     it is a candidate for extension elimination.  */
+  if (GET_CODE (op0) == SUBREG && SUBREG_PROMOTED_VAR_P (op0))
+    op0_ext_redundant = is_assigned_exp_fit_type (treeop0);
+
+  /* If promoted and the value in SUBREG of op1 fits (does not overflow),
+     it is a candidate for extension elimination.  */
+  if (GET_CODE (op1) == SUBREG && SUBREG_PROMOTED_VAR_P (op1))
+    op1_ext_redundant = is_assigned_exp_fit_type (treeop1);
+
+  /* If zero/sign extension is redundant, generate RTL
+     for operands without zero/sign extension.  */
+  if ((op0_ext_redundant || TREE_CODE (treeop0) == INTEGER_CST)
+      && (op1_ext_redundant || TREE_CODE (treeop1) == INTEGER_CST))
+    {
+      if ((TREE_CODE (treeop1) == INTEGER_CST)
+	  && (!mode_signbit_p (GET_MODE (op1), op1)))
+	{
+	  /* First operand is constant and signbit is not set (not
+	     represented in RTL as a negative constant).  */
+	  rtx new_op0 = gen_reg_rtx (GET_MODE (SUBREG_REG (op0)));
+	  emit_move_insn (new_op0, SUBREG_REG (op0));
+	  op0 = new_op0;
+	}
+      else if ((TREE_CODE (treeop0) == INTEGER_CST)
+	       && (!mode_signbit_p (GET_MODE (op0), op0)))
+	{
+	  /* Other operand is constant and signbit is not set (not
+	     represented in RTL as a negative constant).  */
+	  rtx new_op1 = gen_reg_rtx (GET_MODE (SUBREG_REG (op1)));
+
+	  emit_move_insn (new_op1, SUBREG_REG (op1));
+	  op1 = new_op1;
+	}
+      else if ((TREE_CODE (treeop0) != INTEGER_CST)
+	       && (TREE_CODE (treeop1) != INTEGER_CST)
+	       && (GET_MODE (op0) == GET_MODE (op1))
+	       && (GET_MODE (SUBREG_REG (op0)) == GET_MODE (SUBREG_REG (op1))))
+	{
+	  /* If both comapre registers fits SUBREG and of the
+	     same mode.  */
+	  rtx new_op0 = gen_reg_rtx (GET_MODE (SUBREG_REG (op0)));
+	  rtx new_op1 = gen_reg_rtx (GET_MODE (SUBREG_REG (op1)));
+
+	  emit_move_insn (new_op0, SUBREG_REG (op0));
+	  emit_move_insn (new_op1, SUBREG_REG (op1));
+	  op0 = new_op0;
+	  op1 = new_op1;
+	}
+    }
+
   if (TREE_CODE (treeop0) == INTEGER_CST
       && (TREE_CODE (treeop1) != INTEGER_CST
           || (GET_MODE_BITSIZE (mode)