public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
@ 2015-09-01 15:04 Kyrill Tkachov
  2015-09-01 22:13 ` Jeff Law
  0 siblings, 1 reply; 7+ messages in thread
From: Kyrill Tkachov @ 2015-09-01 15:04 UTC (permalink / raw)
  To: GCC Patches
  Cc: Marcus Shawcroft, Richard Earnshaw, Ramana Radhakrishnan,
	James Greenhalgh

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

Hi all,

This first patch introduces the negcc and notcc optabs that should expand to a conditional
negate or a conditional bitwise complement operation.

These are used in ifcvt.c to transform code of the form:
if (test) x = -A; else x = A;
into:
x = A; if (test) x = -x;
where the "if (test) x = -x;" is implemented using the negcc (or notcc in the ~x case)
if such an optab is available. If such an optab is not implemented then no transformation
is performed.  Thus, without patches 2/3 and 3/3 this patch does not impact behaviour on any target.

Bootstrapped and tested as part of the series on arm, aarch64, x86_64.

Ok for trunk?

Thanks,
Kyrill

2015-09-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * ifcvt.c (noce_try_inverse_constants): New function.
     (noce_process_if_block): Call it.
     * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
     * optabs.def (negcc_optab, notcc_optab): Declare.
     * optabs.c (emit_conditional_neg_or_complement): New function.
     * doc/tm.texi (Standard Names): Document negcc, notcc names.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: negnotcc-optabs.patch --]
[-- Type: text/x-patch; name=negnotcc-optabs.patch, Size: 7016 bytes --]

commit a2183218070ed5f2dca0a9651fdb08ce134ba8ee
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Aug 13 18:14:52 2015 +0100

    [optabs][ifcvt][1/3] Define negcc, notcc optabs

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 0bffdc6..5038269 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5791,6 +5791,21 @@ move operand 2 or (operands 2 + operand 3) into operand 0 according to the
 comparison in operand 1.  If the comparison is false, operand 2 is moved into
 operand 0, otherwise (operand 2 + operand 3) is moved.
 
+@cindex @code{neg@var{mode}cc} instruction pattern
+@item @samp{neg@var{mode}cc}
+Similar to @samp{mov@var{mode}cc} but for conditional negation.  Conditionally
+move the negation of operand 2 operand 3 into operand 0 according to the
+comparison in operand 1.  If the comparison is true, the negation of operand 2
+is moved into operand 0, otherwise operand 3 is moved.
+
+@cindex @code{not@var{mode}cc} instruction pattern
+@item @samp{not@var{mode}cc}
+Similar to @samp{neg@var{mode}cc} but for conditional complement.
+Conditionally move the bitwise complement of operand 2 operand 3 into operand 0
+according to the comparison in operand 1.  If the comparison is true,
+the complement of operand 2 is moved into operand 0, otherwise operand 3 is
+moved.
+
 @cindex @code{cstore@var{mode}4} instruction pattern
 @item @samp{cstore@var{mode}4}
 Store zero or nonzero in operand 0 according to whether a comparison
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index d3b244c..aabbde8 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1187,6 +1187,88 @@ noce_try_store_flag (struct noce_if_info *if_info)
     }
 }
 
+
+/* Convert "if (test) x = -A; else x = A" into
+   x = A; if (test) x = -x if the machine can do the
+   conditional negate form of this cheaply.
+   Try this before noce_try_cmove that will just load the
+   immediates into two registers and do a conditional select
+   between them.  If the target has a conditional negate or
+   conditional invert operation we can save a potentially
+   expensive constant synthesis.  */
+
+static bool
+noce_try_inverse_constants (struct noce_if_info *if_info)
+{
+  if (!noce_simple_bbs (if_info))
+    return false;
+
+  if (!CONST_INT_P (if_info->a)
+      || !CONST_INT_P (if_info->b)
+      || !REG_P (if_info->x))
+    return false;
+
+  machine_mode mode = GET_MODE (if_info->x);
+
+  /* If the branch is cheaper than two instructions then this is
+     unlikely to be beneficial.  */
+  if (if_info->branch_cost < 2)
+    return false;
+
+  HOST_WIDE_INT val_a = INTVAL (if_info->a);
+  HOST_WIDE_INT val_b = INTVAL (if_info->b);
+
+  rtx cond = if_info->cond;
+
+  rtx x = if_info->x;
+  rtx target;
+
+  start_sequence ();
+
+  rtx_code code;
+  if (val_a == -val_b)
+    code = NEG;
+  else if (val_a == ~val_b)
+    code = NOT;
+  else
+    {
+      end_sequence ();
+      return false;
+    }
+
+  rtx tmp = gen_reg_rtx (mode);
+  noce_emit_move_insn (tmp, if_info->a);
+
+  target = emit_conditional_neg_or_complement (x, code, mode, cond, tmp, tmp);
+
+  if (target)
+    {
+      rtx_insn *seq = get_insns ();
+
+      if (!seq)
+	{
+	  end_sequence ();
+	  return false;
+	}
+
+      if (target != if_info->x)
+	noce_emit_move_insn (if_info->x, target);
+
+	seq = end_ifcvt_sequence (if_info);
+
+	if (!seq)
+	  return false;
+
+	emit_insn_before_setloc (seq, if_info->jump,
+				 INSN_LOCATION (if_info->insn_a));
+	return true;
+    }
+
+  end_sequence ();
+  return false;
+}
+
+
 /* 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
    and B.  */
@@ -3198,6 +3280,8 @@ noce_process_if_block (struct noce_if_info *if_info)
     goto success;
   if (noce_try_abs (if_info))
     goto success;
+  if (noce_try_inverse_constants (if_info))
+    goto success;
   if (!targetm.have_conditional_execution ()
       && noce_try_store_flag_constants (if_info))
     goto success;
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 97c1d38..dd3ba30 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -4595,6 +4595,56 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
   return NULL_RTX;
 }
 
+
+/* Emit a conditional negate or bitwise complement using the
+   negcc or notcc optabs if available.  Return NULL_RTX if such operations
+   are not available.  Otherwise return the RTX holding the result.
+   TARGET is the desired destination of the result.  COMP is the comparison
+   on which to negate.  If COND is true move into TARGET the negation
+   or bitwise complement of OP1.  Otherwise move OP2 into TARGET.
+   CODE is either NEG or NOT.  MODE is the machine mode in which the
+   operation is performed.  */
+
+rtx
+emit_conditional_neg_or_complement (rtx target, rtx_code code,
+				     machine_mode mode, rtx cond, rtx op1,
+				     rtx op2)
+{
+  optab op = unknown_optab;
+  if (code == NEG)
+    op = negcc_optab;
+  else if (code == NOT)
+    op = notcc_optab;
+  else
+    gcc_unreachable ();
+
+  insn_code icode = direct_optab_handler (op, mode);
+
+  if (icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  if (!target)
+    target = gen_reg_rtx (mode);
+
+  rtx_insn *last = get_last_insn ();
+  struct expand_operand ops[4];
+
+  create_output_operand (&ops[0], target, mode);
+  create_fixed_operand (&ops[1], cond);
+  create_input_operand (&ops[2], op1, mode);
+  create_input_operand (&ops[3], op2, mode);
+
+  if (maybe_expand_insn (icode, 4, ops))
+    {
+      if (ops[0].value != target)
+	convert_move (target, ops[0].value, false);
+
+      return target;
+    }
+  delete_insns_since (last);
+  return NULL_RTX;
+}
+
 /* Return nonzero if a conditional move of mode MODE is supported.
 
    This function is for combine so it can tell whether an insn that looks
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 888b21c..6fad6d9 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -183,6 +183,8 @@ OPTAB_D (reload_out_optab, "reload_out$a")
 
 OPTAB_DC(cbranch_optab, "cbranch$a4", COMPARE)
 OPTAB_D (addcc_optab, "add$acc")
+OPTAB_D (negcc_optab, "neg$acc")
+OPTAB_D (notcc_optab, "not$acc")
 OPTAB_D (movcc_optab, "mov$acc")
 OPTAB_D (cmov_optab, "cmov$a6")
 OPTAB_D (cstore_optab, "cstore$a4")
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 95f5cbc..dbb73d1 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -368,6 +368,10 @@ extern void emit_indirect_jump (rtx);
 rtx emit_conditional_move (rtx, enum rtx_code, rtx, rtx, machine_mode,
 			   rtx, rtx, machine_mode, int);
 
+/* Emit a conditional negate or bitwise complement operation.  */
+rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,
+					 rtx, rtx);
+
 /* Return nonzero if the conditional move is supported.  */
 int can_conditionally_move_p (machine_mode mode);
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
  2015-09-01 15:04 [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs Kyrill Tkachov
@ 2015-09-01 22:13 ` Jeff Law
  2015-09-02 13:43   ` Kyrill Tkachov
  0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2015-09-01 22:13 UTC (permalink / raw)
  To: Kyrill Tkachov, GCC Patches
  Cc: Marcus Shawcroft, Richard Earnshaw, Ramana Radhakrishnan,
	James Greenhalgh

On 09/01/2015 09:04 AM, Kyrill Tkachov wrote:
> Hi all,
>
> This first patch introduces the negcc and notcc optabs that should
> expand to a conditional
> negate or a conditional bitwise complement operation.
>
> These are used in ifcvt.c to transform code of the form:
> if (test) x = -A; else x = A;
> into:
> x = A; if (test) x = -x;
> where the "if (test) x = -x;" is implemented using the negcc (or notcc
> in the ~x case)
> if such an optab is available. If such an optab is not implemented then
> no transformation
> is performed.  Thus, without patches 2/3 and 3/3 this patch does not
> impact behaviour on any target.
>
> Bootstrapped and tested as part of the series on arm, aarch64, x86_64.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2015-09-01  Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
>      * ifcvt.c (noce_try_inverse_constants): New function.
>      (noce_process_if_block): Call it.
>      * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
>      * optabs.def (negcc_optab, notcc_optab): Declare.
>      * optabs.c (emit_conditional_neg_or_complement): New function.
>      * doc/tm.texi (Standard Names): Document negcc, notcc names.
>
> negnotcc-optabs.patch
>
>
> commit a2183218070ed5f2dca0a9651fdb08ce134ba8ee
> Author: Kyrylo Tkachov<kyrylo.tkachov@arm.com>
> Date:   Thu Aug 13 18:14:52 2015 +0100
>
>      [optabs][ifcvt][1/3] Define negcc, notcc optabs
>
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 0bffdc6..5038269 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -5791,6 +5791,21 @@ move operand 2 or (operands 2 + operand 3) into operand 0 according to the
>   comparison in operand 1.  If the comparison is false, operand 2 is moved into
>   operand 0, otherwise (operand 2 + operand 3) is moved.
>
> +@cindex @code{neg@var{mode}cc} instruction pattern
> +@item @samp{neg@var{mode}cc}
> +Similar to @samp{mov@var{mode}cc} but for conditional negation.  Conditionally
> +move the negation of operand 2 operand 3 into operand 0 according to the
> +comparison in operand 1.  If the comparison is true, the negation of operand 2
> +is moved into operand 0, otherwise operand 3 is moved.
> +
> +@cindex @code{not@var{mode}cc} instruction pattern
> +@item @samp{not@var{mode}cc}
> +Similar to @samp{neg@var{mode}cc} but for conditional complement.
> +Conditionally move the bitwise complement of operand 2 operand 3 into operand 0
> +according to the comparison in operand 1.  If the comparison is true,
> +the complement of operand 2 is moved into operand 0, otherwise operand 3 is
> +moved.

"operand 2 operand 3" does not parse.  I think you mean "operand 2 or 
operand 3" in both instances above.  Even that doesn't parse well as 
it's not clear if operand3 or the negation/complement of operand 3 is 
meant.  Can you try to improve the ambiguity of the second sentence in 
each description.

And just a note.  The canonical way to refer to these patterns should be 
negcc/notcc.  That avoids conflicting with the older negscc patterns 
with different semantics that are defined by some ports.  You're already 
using that terminology, so there's nothing to change, I just wanted to 
point it out.



+
> +  rtx_code code;
> +  if (val_a == -val_b)
Do we have to worry about signed overflow here?  I'm thinking 
specifically when val_b is the smallest possible integer representable 
by a HOST_WIDE_INT.  I suspect you may be able to avoid these problems 
with judicious use of the hwi interfaces.


So I think we just need to resolve the doc change and make sure we're 
not triggering undefined behaviour above and this can go forward.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
  2015-09-01 22:13 ` Jeff Law
@ 2015-09-02 13:43   ` Kyrill Tkachov
  2015-09-09 22:01     ` Jeff Law
  0 siblings, 1 reply; 7+ messages in thread
From: Kyrill Tkachov @ 2015-09-02 13:43 UTC (permalink / raw)
  To: Jeff Law, GCC Patches
  Cc: Marcus Shawcroft, Richard Earnshaw, Ramana Radhakrishnan,
	James Greenhalgh

[-- Attachment #1: Type: text/plain, Size: 4945 bytes --]

Hi Jeff,

On 01/09/15 23:13, Jeff Law wrote:
> On 09/01/2015 09:04 AM, Kyrill Tkachov wrote:
>> Hi all,
>>
>> This first patch introduces the negcc and notcc optabs that should
>> expand to a conditional
>> negate or a conditional bitwise complement operation.
>>
>> These are used in ifcvt.c to transform code of the form:
>> if (test) x = -A; else x = A;
>> into:
>> x = A; if (test) x = -x;
>> where the "if (test) x = -x;" is implemented using the negcc (or notcc
>> in the ~x case)
>> if such an optab is available. If such an optab is not implemented then
>> no transformation
>> is performed.  Thus, without patches 2/3 and 3/3 this patch does not
>> impact behaviour on any target.
>>
>> Bootstrapped and tested as part of the series on arm, aarch64, x86_64.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2015-09-01  Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>
>>       * ifcvt.c (noce_try_inverse_constants): New function.
>>       (noce_process_if_block): Call it.
>>       * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
>>       * optabs.def (negcc_optab, notcc_optab): Declare.
>>       * optabs.c (emit_conditional_neg_or_complement): New function.
>>       * doc/tm.texi (Standard Names): Document negcc, notcc names.
>>
>> negnotcc-optabs.patch
>>
>>
>> commit a2183218070ed5f2dca0a9651fdb08ce134ba8ee
>> Author: Kyrylo Tkachov<kyrylo.tkachov@arm.com>
>> Date:   Thu Aug 13 18:14:52 2015 +0100
>>
>>       [optabs][ifcvt][1/3] Define negcc, notcc optabs
>>
>> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
>> index 0bffdc6..5038269 100644
>> --- a/gcc/doc/md.texi
>> +++ b/gcc/doc/md.texi
>> @@ -5791,6 +5791,21 @@ move operand 2 or (operands 2 + operand 3) into operand 0 according to the
>>    comparison in operand 1.  If the comparison is false, operand 2 is moved into
>>    operand 0, otherwise (operand 2 + operand 3) is moved.
>>
>> +@cindex @code{neg@var{mode}cc} instruction pattern
>> +@item @samp{neg@var{mode}cc}
>> +Similar to @samp{mov@var{mode}cc} but for conditional negation.  Conditionally
>> +move the negation of operand 2 operand 3 into operand 0 according to the
>> +comparison in operand 1.  If the comparison is true, the negation of operand 2
>> +is moved into operand 0, otherwise operand 3 is moved.
>> +
>> +@cindex @code{not@var{mode}cc} instruction pattern
>> +@item @samp{not@var{mode}cc}
>> +Similar to @samp{neg@var{mode}cc} but for conditional complement.
>> +Conditionally move the bitwise complement of operand 2 operand 3 into operand 0
>> +according to the comparison in operand 1.  If the comparison is true,
>> +the complement of operand 2 is moved into operand 0, otherwise operand 3 is
>> +moved.
> "operand 2 operand 3" does not parse.  I think you mean "operand 2 or
> operand 3" in both instances above.  Even that doesn't parse well as
> it's not clear if operand3 or the negation/complement of operand 3 is
> meant.  Can you try to improve the ambiguity of the second sentence in
> each description.

You're right, it doesn't parse. I've improved the description.
We either move the negated operand 2 or the unchanged operand 3.

>
> And just a note.  The canonical way to refer to these patterns should be
> negcc/notcc.  That avoids conflicting with the older negscc patterns
> with different semantics that are defined by some ports.  You're already
> using that terminology, so there's nothing to change, I just wanted to
> point it out.
>
>
>
> +
>> +  rtx_code code;
>> +  if (val_a == -val_b)
> Do we have to worry about signed overflow here?  I'm thinking
> specifically when val_b is the smallest possible integer representable
> by a HOST_WIDE_INT.  I suspect you may be able to avoid these problems
> with judicious use of the hwi interfaces.

I understand the issue, but am not sure what hwi interfaces to use here.
Seems that the problem will be if val_b is HOST_WIDE_INT_MIN.
Looking at the definition of abs_hwi in hwint.h before it negates it's argument
it asserts that it's not HOST_WIDE_INT_MIN. I think that's to avoid this exact issue?
If so, I've added a check for HOST_WIDE_INT_MIN which should cover the undefined case
when negating a HOST_WIDE_INT, unless there's something else I'm missing.

>
>
> So I think we just need to resolve the doc change and make sure we're
> not triggering undefined behaviour above and this can go forward.

Thanks, here's the updated patch.
Kyrill

2015-09-02  Kyrylo Tkachov <kyrylo.tkachov@arm.com>

      * ifcvt.c (noce_try_inverse_constants): New function.
      (noce_process_if_block): Call it.
      * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
      * optabs.def (negcc_optab, notcc_optab): Declare.
      * optabs.c (emit_conditional_neg_or_complement): New function.
      * doc/tm.texi (Standard Names): Document negcc, notcc names.


> jeff
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: negnotcc-optabs.patch --]
[-- Type: text/x-patch; name=negnotcc-optabs.patch, Size: 7079 bytes --]

commit e3546b6e9fa772fe15f0a9845dbe429c02f5b327
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Aug 13 18:14:52 2015 +0100

    [optabs][ifcvt][1/3] Define negcc, notcc optabs

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 619259f..c4e43f3 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5791,6 +5791,21 @@ move operand 2 or (operands 2 + operand 3) into operand 0 according to the
 comparison in operand 1.  If the comparison is false, operand 2 is moved into
 operand 0, otherwise (operand 2 + operand 3) is moved.
 
+@cindex @code{neg@var{mode}cc} instruction pattern
+@item @samp{neg@var{mode}cc}
+Similar to @samp{mov@var{mode}cc} but for conditional negation.  Conditionally
+move the negation of operand 2 or the unchanged operand 3 into operand 0
+according to the comparison in operand 1.  If the comparison is true, the negation
+of operand 2 is moved into operand 0, otherwise operand 3 is moved.
+
+@cindex @code{not@var{mode}cc} instruction pattern
+@item @samp{not@var{mode}cc}
+Similar to @samp{neg@var{mode}cc} but for conditional complement.
+Conditionally move the bitwise complement of operand 2 or the unchanged
+operand 3 into operand 0 according to the comparison in operand 1.
+If the comparison is true, the complement of operand 2 is moved into
+operand 0, otherwise operand 3 is moved.
+
 @cindex @code{cstore@var{mode}4} instruction pattern
 @item @samp{cstore@var{mode}4}
 Store zero or nonzero in operand 0 according to whether a comparison
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 157a716..d2f5b66 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1179,6 +1179,88 @@ noce_try_store_flag (struct noce_if_info *if_info)
     }
 }
 
+
+/* Convert "if (test) x = -A; else x = A" into
+   x = A; if (test) x = -x if the machine can do the
+   conditional negate form of this cheaply.
+   Try this before noce_try_cmove that will just load the
+   immediates into two registers and do a conditional select
+   between them.  If the target has a conditional negate or
+   conditional invert operation we can save a potentially
+   expensive constant synthesis.  */
+
+static bool
+noce_try_inverse_constants (struct noce_if_info *if_info)
+{
+  if (!noce_simple_bbs (if_info))
+    return false;
+
+  if (!CONST_INT_P (if_info->a)
+      || !CONST_INT_P (if_info->b)
+      || !REG_P (if_info->x))
+    return false;
+
+  machine_mode mode = GET_MODE (if_info->x);
+
+  /* If the branch is cheaper than two instructions then this is
+     unlikely to be beneficial.  */
+  if (if_info->branch_cost < 2)
+    return false;
+
+  HOST_WIDE_INT val_a = INTVAL (if_info->a);
+  HOST_WIDE_INT val_b = INTVAL (if_info->b);
+
+  rtx cond = if_info->cond;
+
+  rtx x = if_info->x;
+  rtx target;
+
+  start_sequence ();
+
+  rtx_code code;
+  if (val_b != HOST_WIDE_INT_MIN && val_a == -val_b)
+    code = NEG;
+  else if (val_a == ~val_b)
+    code = NOT;
+  else
+    {
+      end_sequence ();
+      return false;
+    }
+
+  rtx tmp = gen_reg_rtx (mode);
+  noce_emit_move_insn (tmp, if_info->a);
+
+  target = emit_conditional_neg_or_complement (x, code, mode, cond, tmp, tmp);
+
+  if (target)
+    {
+      rtx_insn *seq = get_insns ();
+
+      if (!seq)
+	{
+	  end_sequence ();
+	  return false;
+	}
+
+      if (target != if_info->x)
+	noce_emit_move_insn (if_info->x, target);
+
+	seq = end_ifcvt_sequence (if_info);
+
+	if (!seq)
+	  return false;
+
+	emit_insn_before_setloc (seq, if_info->jump,
+				 INSN_LOCATION (if_info->insn_a));
+	return true;
+    }
+
+  end_sequence ();
+  return false;
+}
+
+
 /* 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
    and B.  */
@@ -3190,6 +3272,8 @@ noce_process_if_block (struct noce_if_info *if_info)
     goto success;
   if (noce_try_abs (if_info))
     goto success;
+  if (noce_try_inverse_constants (if_info))
+    goto success;
   if (!targetm.have_conditional_execution ()
       && noce_try_store_flag_constants (if_info))
     goto success;
diff --git a/gcc/optabs.c b/gcc/optabs.c
index e533e6e..aad9f88 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -4597,6 +4597,56 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
   return NULL_RTX;
 }
 
+
+/* Emit a conditional negate or bitwise complement using the
+   negcc or notcc optabs if available.  Return NULL_RTX if such operations
+   are not available.  Otherwise return the RTX holding the result.
+   TARGET is the desired destination of the result.  COMP is the comparison
+   on which to negate.  If COND is true move into TARGET the negation
+   or bitwise complement of OP1.  Otherwise move OP2 into TARGET.
+   CODE is either NEG or NOT.  MODE is the machine mode in which the
+   operation is performed.  */
+
+rtx
+emit_conditional_neg_or_complement (rtx target, rtx_code code,
+				     machine_mode mode, rtx cond, rtx op1,
+				     rtx op2)
+{
+  optab op = unknown_optab;
+  if (code == NEG)
+    op = negcc_optab;
+  else if (code == NOT)
+    op = notcc_optab;
+  else
+    gcc_unreachable ();
+
+  insn_code icode = direct_optab_handler (op, mode);
+
+  if (icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  if (!target)
+    target = gen_reg_rtx (mode);
+
+  rtx_insn *last = get_last_insn ();
+  struct expand_operand ops[4];
+
+  create_output_operand (&ops[0], target, mode);
+  create_fixed_operand (&ops[1], cond);
+  create_input_operand (&ops[2], op1, mode);
+  create_input_operand (&ops[3], op2, mode);
+
+  if (maybe_expand_insn (icode, 4, ops))
+    {
+      if (ops[0].value != target)
+	convert_move (target, ops[0].value, false);
+
+      return target;
+    }
+  delete_insns_since (last);
+  return NULL_RTX;
+}
+
 /* Return nonzero if a conditional move of mode MODE is supported.
 
    This function is for combine so it can tell whether an insn that looks
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 888b21c..6fad6d9 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -183,6 +183,8 @@ OPTAB_D (reload_out_optab, "reload_out$a")
 
 OPTAB_DC(cbranch_optab, "cbranch$a4", COMPARE)
 OPTAB_D (addcc_optab, "add$acc")
+OPTAB_D (negcc_optab, "neg$acc")
+OPTAB_D (notcc_optab, "not$acc")
 OPTAB_D (movcc_optab, "mov$acc")
 OPTAB_D (cmov_optab, "cmov$a6")
 OPTAB_D (cstore_optab, "cstore$a4")
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 95f5cbc..dbb73d1 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -368,6 +368,10 @@ extern void emit_indirect_jump (rtx);
 rtx emit_conditional_move (rtx, enum rtx_code, rtx, rtx, machine_mode,
 			   rtx, rtx, machine_mode, int);
 
+/* Emit a conditional negate or bitwise complement operation.  */
+rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,
+					 rtx, rtx);
+
 /* Return nonzero if the conditional move is supported.  */
 int can_conditionally_move_p (machine_mode mode);
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
  2015-09-02 13:43   ` Kyrill Tkachov
@ 2015-09-09 22:01     ` Jeff Law
  2015-09-10  8:28       ` Kyrill Tkachov
  0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2015-09-09 22:01 UTC (permalink / raw)
  To: Kyrill Tkachov, GCC Patches
  Cc: Marcus Shawcroft, Richard Earnshaw, Ramana Radhakrishnan,
	James Greenhalgh

On 09/02/2015 07:43 AM, Kyrill Tkachov wrote:

>>> +  rtx_code code; +  if (val_a == -val_b)
>> Do we have to worry about signed overflow here?  I'm thinking
>> specifically when val_b is the smallest possible integer
>> representable by a HOST_WIDE_INT.  I suspect you may be able to
>> avoid these problems with judicious use of the hwi interfaces.
>
> I understand the issue, but am not sure what hwi interfaces to use
> here. Seems that the problem will be if val_b is HOST_WIDE_INT_MIN.
Right.  When val_b is HOST_WIDE_INT_MIN, negating it will overflow.

> Looking at the definition of abs_hwi in hwint.h before it negates
> it's argument it asserts that it's not HOST_WIDE_INT_MIN. I think
> that's to avoid this exact issue? If so, I've added a check for
> HOST_WIDE_INT_MIN which should cover the undefined case when negating
> a HOST_WIDE_INT, unless there's something else I'm missing.
That should be sufficient.

2015-09-02  Kyrylo Tkachov <kyrylo.tkachov@arm.com>

      * ifcvt.c (noce_try_inverse_constants): New function.
      (noce_process_if_block): Call it.
      * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
      * optabs.def (negcc_optab, notcc_optab): Declare.
      * optabs.c (emit_conditional_neg_or_complement): New function.
      * doc/tm.texi (Standard Names): Document negcc, notcc names.

OK for the trunk.

Thanks for your patience.

Jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
  2015-09-09 22:01     ` Jeff Law
@ 2015-09-10  8:28       ` Kyrill Tkachov
  0 siblings, 0 replies; 7+ messages in thread
From: Kyrill Tkachov @ 2015-09-10  8:28 UTC (permalink / raw)
  To: Jeff Law, GCC Patches
  Cc: Marcus Shawcroft, Richard Earnshaw, Ramana Radhakrishnan,
	James Greenhalgh


On 09/09/15 22:51, Jeff Law wrote:
> On 09/02/2015 07:43 AM, Kyrill Tkachov wrote:
>
>>>> +  rtx_code code; +  if (val_a == -val_b)
>>> Do we have to worry about signed overflow here?  I'm thinking
>>> specifically when val_b is the smallest possible integer
>>> representable by a HOST_WIDE_INT.  I suspect you may be able to
>>> avoid these problems with judicious use of the hwi interfaces.
>> I understand the issue, but am not sure what hwi interfaces to use
>> here. Seems that the problem will be if val_b is HOST_WIDE_INT_MIN.
> Right.  When val_b is HOST_WIDE_INT_MIN, negating it will overflow.
>
>> Looking at the definition of abs_hwi in hwint.h before it negates
>> it's argument it asserts that it's not HOST_WIDE_INT_MIN. I think
>> that's to avoid this exact issue? If so, I've added a check for
>> HOST_WIDE_INT_MIN which should cover the undefined case when negating
>> a HOST_WIDE_INT, unless there's something else I'm missing.
> That should be sufficient.
>
> 2015-09-02  Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
>        * ifcvt.c (noce_try_inverse_constants): New function.
>        (noce_process_if_block): Call it.
>        * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
>        * optabs.def (negcc_optab, notcc_optab): Declare.
>        * optabs.c (emit_conditional_neg_or_complement): New function.
>        * doc/tm.texi (Standard Names): Document negcc, notcc names.
>
> OK for the trunk.

Thanks for the review.
I'll commit it if/when the aarch64 and arm implementations of the hooks
in patches 2/3 and 3/3 are approved.

Kyrill

>
> Thanks for your patience.
>
> Jeff
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
  2015-11-09 12:23 Kyrill Tkachov
@ 2015-11-09 23:11 ` Jeff Law
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2015-11-09 23:11 UTC (permalink / raw)
  To: Kyrill Tkachov, GCC Patches

On 11/09/2015 05:23 AM, Kyrill Tkachov wrote:
> Hi all,
>
> This is a rebase of the patch I posted at:
> https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00154.html
>
> The patch has been ok'd by Jeff but I wanted to hold off committing it
> until
> my fixes for the ifcvt regressions on sparc and x86_64 were fixed.
>
> The rebase conflicts were due to Richard's optabs splitting patch.
>
> I've also noticed that in my original patch I had a comparison of branch
> cost with
> the magic number '2'. I removed it from this version as it's not really
> meaningful.
> The transformation this patch enables is, at the moment, only supported
> for arm
> and aarch64 where it is always beneficial. If/when we have a proper
> ifcvt costing
> model (perhaps for GCC 7?) we'll update this accordingly if needed.
>
> Jeff, sorry for taking so long to commit this, I just wanted to fix the
> other
> ifcvt fallout before proceeding with more new functionality.
> I have also uncovered a bug in the arm implementation of these optabs
> (patch 3/3 in the series), so I'll post an updated version of that patch
> as well soon.
>
> Ok to commit this updated version instead?
>
> Bootstrapped and tested on arm, aarch64 and x86_64.
> It has been sitting in my tree for a couple of months now with no issues.
>
> Thanks,
> Kyrill
>
> 2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * ifcvt.c (noce_try_inverse_constants): New function.
>      (noce_process_if_block): Call it.
>      * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
>      * optabs.def (negcc_optab, notcc_optab): Declare.
>      * optabs.c (emit_conditional_neg_or_complement): New function.
>      * doc/tm.texi (Standard Names): Document negcc, notcc names.
Thanks for addressing the ifcvt fallout first, then making sure this 
didn't get lost.

Yes, this is fine for the trunk.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs
@ 2015-11-09 12:23 Kyrill Tkachov
  2015-11-09 23:11 ` Jeff Law
  0 siblings, 1 reply; 7+ messages in thread
From: Kyrill Tkachov @ 2015-11-09 12:23 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jeff Law

[-- Attachment #1: Type: text/plain, Size: 1656 bytes --]

Hi all,

This is a rebase of the patch I posted at:
https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00154.html

The patch has been ok'd by Jeff but I wanted to hold off committing it until
my fixes for the ifcvt regressions on sparc and x86_64 were fixed.

The rebase conflicts were due to Richard's optabs splitting patch.

I've also noticed that in my original patch I had a comparison of branch cost with
the magic number '2'. I removed it from this version as it's not really meaningful.
The transformation this patch enables is, at the moment, only supported for arm
and aarch64 where it is always beneficial. If/when we have a proper ifcvt costing
model (perhaps for GCC 7?) we'll update this accordingly if needed.

Jeff, sorry for taking so long to commit this, I just wanted to fix the other
ifcvt fallout before proceeding with more new functionality.
I have also uncovered a bug in the arm implementation of these optabs
(patch 3/3 in the series), so I'll post an updated version of that patch as well soon.

Ok to commit this updated version instead?

Bootstrapped and tested on arm, aarch64 and x86_64.
It has been sitting in my tree for a couple of months now with no issues.

Thanks,
Kyrill

2015-11-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * ifcvt.c (noce_try_inverse_constants): New function.
     (noce_process_if_block): Call it.
     * optabs.h (emit_conditional_neg_or_complement): Declare prototype.
     * optabs.def (negcc_optab, notcc_optab): Declare.
     * optabs.c (emit_conditional_neg_or_complement): New function.
     * doc/tm.texi (Standard Names): Document negcc, notcc names.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: negnotcc-optabs.patch --]
[-- Type: text/x-patch; name=negnotcc-optabs.patch, Size: 6882 bytes --]

commit 93cd987e9ab02ac68b44b2470bb5c4c6345efeca
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Aug 13 18:14:52 2015 +0100

    [optabs][ifcvt][1/3] Define negcc, notcc optabs

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 619259f..c4e43f3 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5791,6 +5791,21 @@ move operand 2 or (operands 2 + operand 3) into operand 0 according to the
 comparison in operand 1.  If the comparison is false, operand 2 is moved into
 operand 0, otherwise (operand 2 + operand 3) is moved.
 
+@cindex @code{neg@var{mode}cc} instruction pattern
+@item @samp{neg@var{mode}cc}
+Similar to @samp{mov@var{mode}cc} but for conditional negation.  Conditionally
+move the negation of operand 2 or the unchanged operand 3 into operand 0
+according to the comparison in operand 1.  If the comparison is true, the negation
+of operand 2 is moved into operand 0, otherwise operand 3 is moved.
+
+@cindex @code{not@var{mode}cc} instruction pattern
+@item @samp{not@var{mode}cc}
+Similar to @samp{neg@var{mode}cc} but for conditional complement.
+Conditionally move the bitwise complement of operand 2 or the unchanged
+operand 3 into operand 0 according to the comparison in operand 1.
+If the comparison is true, the complement of operand 2 is moved into
+operand 0, otherwise operand 3 is moved.
+
 @cindex @code{cstore@var{mode}4} instruction pattern
 @item @samp{cstore@var{mode}4}
 Store zero or nonzero in operand 0 according to whether a comparison
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c
index 157a716..1e773d8 100644
--- a/gcc/ifcvt.c
+++ b/gcc/ifcvt.c
@@ -1179,6 +1179,83 @@ noce_try_store_flag (struct noce_if_info *if_info)
     }
 }
 
+
+/* Convert "if (test) x = -A; else x = A" into
+   x = A; if (test) x = -x if the machine can do the
+   conditional negate form of this cheaply.
+   Try this before noce_try_cmove that will just load the
+   immediates into two registers and do a conditional select
+   between them.  If the target has a conditional negate or
+   conditional invert operation we can save a potentially
+   expensive constant synthesis.  */
+
+static bool
+noce_try_inverse_constants (struct noce_if_info *if_info)
+{
+  if (!noce_simple_bbs (if_info))
+    return false;
+
+  if (!CONST_INT_P (if_info->a)
+      || !CONST_INT_P (if_info->b)
+      || !REG_P (if_info->x))
+    return false;
+
+  machine_mode mode = GET_MODE (if_info->x);
+
+  HOST_WIDE_INT val_a = INTVAL (if_info->a);
+  HOST_WIDE_INT val_b = INTVAL (if_info->b);
+
+  rtx cond = if_info->cond;
+
+  rtx x = if_info->x;
+  rtx target;
+
+  start_sequence ();
+
+  rtx_code code;
+  if (val_b != HOST_WIDE_INT_MIN && val_a == -val_b)
+    code = NEG;
+  else if (val_a == ~val_b)
+    code = NOT;
+  else
+    {
+      end_sequence ();
+      return false;
+    }
+
+  rtx tmp = gen_reg_rtx (mode);
+  noce_emit_move_insn (tmp, if_info->a);
+
+  target = emit_conditional_neg_or_complement (x, code, mode, cond, tmp, tmp);
+
+  if (target)
+    {
+      rtx_insn *seq = get_insns ();
+
+      if (!seq)
+	{
+	  end_sequence ();
+	  return false;
+	}
+
+      if (target != if_info->x)
+	noce_emit_move_insn (if_info->x, target);
+
+	seq = end_ifcvt_sequence (if_info);
+
+	if (!seq)
+	  return false;
+
+	emit_insn_before_setloc (seq, if_info->jump,
+				 INSN_LOCATION (if_info->insn_a));
+	return true;
+    }
+
+  end_sequence ();
+  return false;
+}
+
+
 /* 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
    and B.  */
@@ -3190,6 +3267,8 @@ noce_process_if_block (struct noce_if_info *if_info)
     goto success;
   if (noce_try_abs (if_info))
     goto success;
+  if (noce_try_inverse_constants (if_info))
+    goto success;
   if (!targetm.have_conditional_execution ()
       && noce_try_store_flag_constants (if_info))
     goto success;
diff --git a/gcc/optabs.c b/gcc/optabs.c
index c49d66b..e388d2a 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -4210,6 +4210,56 @@ emit_conditional_move (rtx target, enum rtx_code code, rtx op0, rtx op1,
   return NULL_RTX;
 }
 
+
+/* Emit a conditional negate or bitwise complement using the
+   negcc or notcc optabs if available.  Return NULL_RTX if such operations
+   are not available.  Otherwise return the RTX holding the result.
+   TARGET is the desired destination of the result.  COMP is the comparison
+   on which to negate.  If COND is true move into TARGET the negation
+   or bitwise complement of OP1.  Otherwise move OP2 into TARGET.
+   CODE is either NEG or NOT.  MODE is the machine mode in which the
+   operation is performed.  */
+
+rtx
+emit_conditional_neg_or_complement (rtx target, rtx_code code,
+				     machine_mode mode, rtx cond, rtx op1,
+				     rtx op2)
+{
+  optab op = unknown_optab;
+  if (code == NEG)
+    op = negcc_optab;
+  else if (code == NOT)
+    op = notcc_optab;
+  else
+    gcc_unreachable ();
+
+  insn_code icode = direct_optab_handler (op, mode);
+
+  if (icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  if (!target)
+    target = gen_reg_rtx (mode);
+
+  rtx_insn *last = get_last_insn ();
+  struct expand_operand ops[4];
+
+  create_output_operand (&ops[0], target, mode);
+  create_fixed_operand (&ops[1], cond);
+  create_input_operand (&ops[2], op1, mode);
+  create_input_operand (&ops[3], op2, mode);
+
+  if (maybe_expand_insn (icode, 4, ops))
+    {
+      if (ops[0].value != target)
+	convert_move (target, ops[0].value, false);
+
+      return target;
+    }
+  delete_insns_since (last);
+  return NULL_RTX;
+}
+
 /* Emit a conditional addition instruction if the machine supports one for that
    condition and machine mode.
 
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 888b21c..6fad6d9 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -183,6 +183,8 @@ OPTAB_D (reload_out_optab, "reload_out$a")
 
 OPTAB_DC(cbranch_optab, "cbranch$a4", COMPARE)
 OPTAB_D (addcc_optab, "add$acc")
+OPTAB_D (negcc_optab, "neg$acc")
+OPTAB_D (notcc_optab, "not$acc")
 OPTAB_D (movcc_optab, "mov$acc")
 OPTAB_D (cmov_optab, "cmov$a6")
 OPTAB_D (cstore_optab, "cstore$a4")
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 3f29d1b..5e6fe11 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -259,6 +259,10 @@ extern void emit_indirect_jump (rtx);
 rtx emit_conditional_move (rtx, enum rtx_code, rtx, rtx, machine_mode,
 			   rtx, rtx, machine_mode, int);
 
+/* Emit a conditional negate or bitwise complement operation.  */
+rtx emit_conditional_neg_or_complement (rtx, rtx_code, machine_mode, rtx,
+					 rtx, rtx);
+
 rtx emit_conditional_add (rtx, enum rtx_code, rtx, rtx, machine_mode,
 			  rtx, rtx, machine_mode, int);
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-11-09 23:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-01 15:04 [PATCH][optabs][ifcvt][1/3] Define negcc, notcc optabs Kyrill Tkachov
2015-09-01 22:13 ` Jeff Law
2015-09-02 13:43   ` Kyrill Tkachov
2015-09-09 22:01     ` Jeff Law
2015-09-10  8:28       ` Kyrill Tkachov
2015-11-09 12:23 Kyrill Tkachov
2015-11-09 23:11 ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).