Re: [PATCH][ARM][3/3] Implement negsicc, notsicc optabs

[Ramana Radhakrishnan]

On 01/09/15 16:10, Kyrill Tkachov wrote:
> [Resending with patch attached]
> 
> Hi all,
> 
> This third patch implements the new optabs for arm.
> Conveniently, we can reuse the recently refactored *if_neg_move pattern
> and extend it to cover the conditional NOT case.
> Although arm has conditional execution capabilities I have found that
> performing the transformation from patch 1/3 early on benefits arm as well.
> For example for code:
> int
> foonegsi (int a)
> {
>   return a ? 25089992 : -25089992;
> }
> 
> we currently generate:
> movwr2, #55240
> movwr3, #10296
> cmp r0, #0
> movtr2, 382
> movtr3, 65153
> movne   r0, r2
> moveq   r0, r3
> bx
> 
> whereas with this patch we generate:
> movwr3, #10296
> cmp r0, #0
> movtr3, 65153
> mov r0, r3
> rsbne   r0, r3, #0
> bx  lr
> 
> 
> In SPEC2006 this doesn't trigger very often, so there were minimal code 
> differences,
> but overall I claim this patch is an improvement.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Ok for trunk if the midend changes in 1/3 are approved?


OK.

Thanks,
Ramana
> 
> Thanks,
> Kyrill
> 
> 2015-08-20  Kyrylo Tkachov  
> 
> * config/arm/arm.md (sicc): New define_expand.
> (*if_neg_move): Rename to...
> (*if__move): ... This.  Use NOT_NEG code iterator.
> * config/arm/iterators.md (NOT_NEG): New code iterator.
> (NOT_NEG_op): New code attribute.
> 
> 2015-08-20  Kyrylo Tkachov  
> 
> * gcc.target/arm/cond_op_imm_1.c: New test.

[PATCH][ARM][3/3] Implement negsicc, notsicc optabs

[Kyrill Tkachov]

Hi all,

This third patch implements the new optabs for arm.
Conveniently, we can reuse the recently refactored *if_neg_move pattern
and extend it to cover the conditional NOT case.
Although arm has conditional execution capabilities I have found that
performing the transformation from patch 1/3 early on benefits arm as well.
For example for code:
int
foonegsi (int a)
{
  return a ? 25089992 : -25089992;
}

we currently generate:
movwr2, #55240
movwr3, #10296
cmp r0, #0
movtr2, 382
movtr3, 65153
movne   r0, r2
moveq   r0, r3
bx

whereas with this patch we generate:
movwr3, #10296
cmp r0, #0
movtr3, 65153
mov r0, r3
rsbne   r0, r3, #0
bx  lr


In SPEC2006 this doesn't trigger very often, so there were minimal code 
differences,
but overall I claim this patch is an improvement.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk if the midend changes in 1/3 are approved?

Thanks,
Kyrill

2015-09-01  Kyrylo Tkachov  

* config/arm/arm.md (sicc): New define_expand.
(*if_neg_move): Rename to...
(*if__move): ... This.  Use NOT_NEG code iterator.
* config/arm/iterators.md (NOT_NEG): New code iterator.
(NOT_NEG_op): New code attribute.

2015-09-01  Kyrylo Tkachov  

* gcc.target/arm/cond_op_imm_1.c: New test.

Re: [PATCH][ARM][3/3] Implement negsicc, notsicc optabs

[pinskia]

> On Sep 1, 2015, at 11:04 PM, Kyrill Tkachov  wrote:
> 
> Hi all,
> 
> This third patch implements the new optabs for arm.
> Conveniently, we can reuse the recently refactored *if_neg_move pattern
> and extend it to cover the conditional NOT case.
> Although arm has conditional execution capabilities I have found that
> performing the transformation from patch 1/3 early on benefits arm as well.
> For example for code:
> int
> foonegsi (int a)
> {
>  return a ? 25089992 : -25089992;
> }
> 
> we currently generate:
>movwr2, #55240
>movwr3, #10296
>cmp r0, #0
>movtr2, 382
>movtr3, 65153
>movne   r0, r2
>moveq   r0, r3
>bx
> 
> whereas with this patch we generate:
>movwr3, #10296
>cmp r0, #0
>movtr3, 65153
>mov r0, r3
>rsbne   r0, r3, #0
>bx  lr
> 
> 
> In SPEC2006 this doesn't trigger very often, so there were minimal code 
> differences,
> but overall I claim this patch is an improvement.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Ok for trunk if the midend changes in 1/3 are approved?

ENOPATCH

> 
> Thanks,
> Kyrill
> 
> 2015-09-01  Kyrylo Tkachov  
> 
>* config/arm/arm.md (sicc): New define_expand.
>(*if_neg_move): Rename to...
>(*if__move): ... This.  Use NOT_NEG code iterator.
>* config/arm/iterators.md (NOT_NEG): New code iterator.
>(NOT_NEG_op): New code attribute.
> 
> 2015-09-01  Kyrylo Tkachov  
> 
>* gcc.target/arm/cond_op_imm_1.c: New test.
> 

[PATCH][ARM][3/3] Implement negsicc, notsicc optabs

[Kyrill Tkachov]

[Resending with patch attached]

Hi all,

This third patch implements the new optabs for arm.
Conveniently, we can reuse the recently refactored *if_neg_move pattern
and extend it to cover the conditional NOT case.
Although arm has conditional execution capabilities I have found that
performing the transformation from patch 1/3 early on benefits arm as well.
For example for code:
int
foonegsi (int a)
{
  return a ? 25089992 : -25089992;
}

we currently generate:
movwr2, #55240
movwr3, #10296
cmp r0, #0
movtr2, 382
movtr3, 65153
movne   r0, r2
moveq   r0, r3
bx

whereas with this patch we generate:
movwr3, #10296
cmp r0, #0
movtr3, 65153
mov r0, r3
rsbne   r0, r3, #0
bx  lr


In SPEC2006 this doesn't trigger very often, so there were minimal code 
differences,
but overall I claim this patch is an improvement.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk if the midend changes in 1/3 are approved?

Thanks,
Kyrill

2015-08-20  Kyrylo Tkachov  

* config/arm/arm.md (sicc): New define_expand.
(*if_neg_move): Rename to...
(*if__move): ... This.  Use NOT_NEG code iterator.
* config/arm/iterators.md (NOT_NEG): New code iterator.
(NOT_NEG_op): New code attribute.

2015-08-20  Kyrylo Tkachov  

* gcc.target/arm/cond_op_imm_1.c: New test.
commit 42353418133073915f23e910d0cd63299fbbffcf
Author: Kyrylo Tkachov 
Date:   Fri Aug 14 13:42:51 2015 +0100

[ARM][3/3] Implement negsicc, notsicc optabs

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 3e7d97e..6f61bb3 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -10102,18 +10102,39 @@ (define_insn "*ifcompare_neg_move"
(set_attr "type" "multiple")]
 )
 
-(define_insn_and_split "*if_neg_move"
+;; The negsicc and notsicc optabs.
+(define_expand "sicc"
+  [(set (match_operand:SI 0 "s_register_operand" "")
+	(if_then_else:SI (match_operand 1 "arm_comparison_operator" "")
+			  (NOT_NEG:SI (match_operand:SI 2 "s_register_operand" ""))
+			  (match_operand:SI 3 "s_register_operand" "")))]
+  "TARGET_32BIT"
+  {
+rtx ccreg;
+enum rtx_code code = GET_CODE (operands[1]);
+
+if (code == UNEQ || code == LTGT)
+  FAIL;
+
+ccreg = arm_gen_compare_reg (code, XEXP (operands[1], 0),
+  XEXP (operands[1], 1), NULL);
+operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+  }
+)
+
+
+(define_insn_and_split "*if__move"
   [(set (match_operand:SI 0 "s_register_operand" "=l,r")
 	(if_then_else:SI
 	 (match_operator 4 "arm_comparison_operator"
 	  [(match_operand 3 "cc_register" "") (const_int 0)])
-	 (neg:SI (match_operand:SI 2 "s_register_operand" "l,r"))
+	 (NOT_NEG:SI (match_operand:SI 2 "s_register_operand" "l,r"))
 	 (match_operand:SI 1 "s_register_operand" "0,0")))]
   "TARGET_32BIT"
   "#"
   "&& reload_completed"
   [(cond_exec (match_op_dup 4 [(match_dup 3) (const_int 0)])
-	  (set (match_dup 0) (neg:SI (match_dup 2]
+	  (set (match_dup 0) (NOT_NEG:SI (match_dup 2]
   ""
   [(set_attr "conds" "use")
(set_attr "length" "4")
diff --git a/gcc/config/arm/iterators.md b/gcc/config/arm/iterators.md
index 1e7f3f1..db61613 100644
--- a/gcc/config/arm/iterators.md
+++ b/gcc/config/arm/iterators.md
@@ -193,6 +193,9 @@ (define_code_iterator COMPARISONS [eq gt ge le lt])
 ;; A list of ...
 (define_code_iterator IOR_XOR [ior xor])
 
+;; Bitwise complement and negation
+(define_code_iterator NOT_NEG [not neg])
+
 ;; Operations on two halves of a quadword vector.
 (define_code_iterator VQH_OPS [plus smin smax umin umax])
 
@@ -629,6 +632,8 @@ (define_code_attr VQH_type [(plus "add") (smin "minmax") (smax "minmax")
 (define_code_attr VQH_sign [(plus "i") (smin "s") (smax "s") (umin "u")
 (umax "u")])
 
+(define_code_attr NOT_NEG_op [(not "not") (neg "neg")])
+
 (define_code_attr cnb [(ltu "CC_C") (geu "CC")])
 (define_code_attr optab [(ltu "ltu") (geu "geu")])
 
diff --git a/gcc/testsuite/gcc.target/arm/cond_op_imm_1.c b/gcc/testsuite/gcc.target/arm/cond_op_imm_1.c
new file mode 100644
index 000..9d335e2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cond_op_imm_1.c
@@ -0,0 +1,42 @@
+/* { dg-do run } */
+/* { dg-options "-save-temps -O2 -fno-inline" } */
+/* { dg-require-effective-target arm_cond_exec } */
+
+extern void abort (void);
+
+#define N 25089992
+
+int
+foonegsi (int a)
+{
+  return a ? N : -N;
+}
+
+int
+fooinvsi (int a)
+{
+  return a ? N : ~N;
+}
+
+
+
+int
+main (void)
+{
+  if (foonegsi (1) != N)
+abort ();
+
+  if (foonegsi (0) != -N)
+abort ();
+
+  if (fooinvsi (1) != N)
+abort ();
+
+  if (fooinvsi (0) != ~N)
+abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler "rsbne" } } */
+/* { dg-final { scan-assembler "mvnne" } } */