diff mbox

[RTL,ifcvt] Transform (X == CST) ? -CST : Y into (X == CST) ? -X : Y when conditional negation is available

Message ID 57EB83EB.9070103@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov Sept. 28, 2016, 8:48 a.m. UTC 
Hi all,

This patch tries to avoid materialising an immediate
when comparison has shown that it is already loaded.
This is performed during ifcvt and only when the target has the conditional negate
or inverse optabs, which for now is only aarch64.

Thus for the code:
int
foo (int a, int b)
{
   return a == 5 ? -5 : b;
}

we currently emit on aarch64:
foo:
         mov     w2, -5
         cmp     w0, 5
         csel    w0, w1, w2, ne
         ret

but with this patch we emit:
foo:
         cmp     w0, 5
         csneg   w0, w1, w0, ne
         ret

Bootstrapped and tested on arm, aarch64, x86_64.

Ok for trunk?

Thanks,
Kyrill

P.S. The simpler form of the transformation: X == CST ? CST : Y --> X == CST ? X : Y
could also be beneficial IMO but I found that it can cause bad codegen in combine
due to extending the live range of X. I'm still working on a way to work around that,
but that is a separate piece of work anyway.

2016-09-28  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * ifcvt.c (noce_try_avoid_const_materialization): New function.
     (noce_process_if_block): Use it.

2016-09-28  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/aarch64/ifcvt_avoid_const_materialization_1.c: New test.

Comments

Jeff Law Sept. 28, 2016, 4:07 p.m. UTC | #1 
On 09/28/2016 02:48 AM, Kyrill Tkachov wrote:
> Hi all,
>
> This patch tries to avoid materialising an immediate
> when comparison has shown that it is already loaded.
> This is performed during ifcvt and only when the target has the
> conditional negate
> or inverse optabs, which for now is only aarch64.
>
> Thus for the code:
> int
> foo (int a, int b)
> {
>   return a == 5 ? -5 : b;
> }
>
> we currently emit on aarch64:
> foo:
>         mov     w2, -5
>         cmp     w0, 5
>         csel    w0, w1, w2, ne
>         ret
>
> but with this patch we emit:
> foo:
>         cmp     w0, 5
>         csneg   w0, w1, w0, ne
>         ret
>
> Bootstrapped and tested on arm, aarch64, x86_64.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> P.S. The simpler form of the transformation: X == CST ? CST : Y --> X ==
> CST ? X : Y
> could also be beneficial IMO but I found that it can cause bad codegen
> in combine
> due to extending the live range of X. I'm still working on a way to work
> around that,
> but that is a separate piece of work anyway.
FWIW, the simpler form of the transformation is already done just prior 
to leaving SSA form in tree-ssa-uncprop.c.  So there may not be a ton of 
opportunities for the simpler form in the RTL optimizers.

My only concern here is this transformation isn't significantly 
different than the simpler form, which is apparently causing some kind 
of combine problem.

I'd like to understand the combine problem better before giving final 
approval to this patch.


Jeff
diff mbox

Patch

diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 24542f008485e6c28e068030fa301f2ce040efc1..203cfe98f82a49c35520a2cb16d2fa09d4c85c72 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1313,6 +1313,84 @@  noce_try_inverse_constants (struct noce_if_info *if_info)
   return false;
 }
 
+/* Try to avoid materializing a constant if we know it's in one of the
+   registers.  For example:
+   (X == CST) ? -CST : Y --> (X == CST) ? -X : Y.
+   Do this only if conditional negation is available.
+   Similar for bitwise NOT.  */
+
+static bool
+noce_try_avoid_const_materialization (struct noce_if_info *if_info)
+{
+  if (!noce_simple_bbs (if_info))
+    return false;
+
+  rtx cond = if_info->cond;
+  rtx a = if_info->a;
+  rtx b = if_info->b;
+  rtx_code code = GET_CODE (cond);
+  machine_mode mode = GET_MODE (if_info->x);
+
+  if (!(code == EQ || code == NE)
+      || !REG_P (XEXP (cond, 0))
+      || !REG_P (if_info->x)
+      || GET_MODE (XEXP (cond, 0)) != mode
+      || !CONST_INT_P (XEXP (cond, 1)))
+    return false;
+
+  rtx cst = XEXP (cond, 1);
+  if (cst == CONST0_RTX (mode))
+    return false;
+
+  rtx non_cst = XEXP (cond, 0);
+  rtx eq_side = code == EQ ? b : a;
+  if (!CONST_INT_P (eq_side))
+    return false;
+
+  HOST_WIDE_INT cstval = INTVAL (cst);
+  HOST_WIDE_INT eq_side_val = INTVAL (eq_side);
+
+  rtx_code op_code;
+  if (eq_side_val == ~cstval)
+    op_code = NOT;
+  else if (eq_side_val != HOST_WIDE_INT_MIN && (cstval == -eq_side_val))
+    op_code = NEG;
+  else
+    return false;
+
+  /* By the rules of the negcc/notcc optabs must happen when the COND is true,
+     in this case when register in COND is equal to CST so always set the
+     comparison to EQ.  */
+  if (code == NE)
+    {
+      a = non_cst;
+      cond = gen_rtx_fmt_ee (EQ, GET_MODE (cond), non_cst, cst);
+    }
+  else
+    b = non_cst;
+
+  start_sequence ();
+  rtx target
+    = emit_conditional_neg_or_complement (if_info->x, op_code, mode,
+					   cond, a, b);
+  if (!target)
+    {
+      end_sequence ();
+      return false;
+    }
+
+  if (target != if_info->x)
+    noce_emit_move_insn (if_info->x, target);
+
+  rtx_insn *seq = end_ifcvt_sequence (if_info);
+  if (!seq)
+    return false;
+
+   emit_insn_before_setloc (seq, if_info->jump,
+			     INSN_LOCATION (if_info->insn_a));
+  if_info->transform_name = "noce_try_avoid_const_materialization";
+  return true;
+}
 
 /* Convert "if (test) x = a; else x = b", for A and B constant.
    Also allow A = y + c1, B = y + c2, with a common y between A
@@ -3606,6 +3684,8 @@  noce_process_if_block (struct noce_if_info *if_info)
     goto success;
   if (noce_try_inverse_constants (if_info))
     goto success;
+  if (noce_try_avoid_const_materialization (if_info))
+    goto success;
   if (!targetm.have_conditional_execution ()
       && noce_try_store_flag_constants (if_info))
     goto success;
diff --git a/gcc/testsuite/gcc.target/aarch64/ifcvt_avoid_const_materialization_1.c b/gcc/testsuite/gcc.target/aarch64/ifcvt_avoid_const_materialization_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..b2a05eaff606a54afc40c3899ad5926c889283dc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/ifcvt_avoid_const_materialization_1.c
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* Check that we avoid moving the immediate into a register
+   if comparison has shown that the inverse or negated form is
+   already in one of the registers.  */
+
+int
+foo (int a, int b)
+{
+  return a == 5 ? -5 : b;
+}
+
+int
+bar (int a, int b)
+{
+  return a != 5 ? b : ~5;
+}
+
+/* { dg-final { scan-assembler-not "mov\\tw\[0-9\]+" } } */
+/* { dg-final { scan-assembler-times "csneg\\tw\[0-9\]+" 1 } } */
+/* { dg-final { scan-assembler-times "csinv\\tw\[0-9\]+" 1 } } */