public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/5][Arm] New pattern for CSINV instructions
@ 2020-08-04 16:11 Omar Tahir
  2020-08-05  8:45 ` Kyrylo Tkachov
  0 siblings, 1 reply; 4+ messages in thread
From: Omar Tahir @ 2020-08-04 16:11 UTC (permalink / raw)
  To: Kyrylo Tkachov, nickc, Ramana Radhakrishnan, Richard Earnshaw,
	gcc-patches

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

This patch adds a new pattern, *thumb2_csinv, for generating CSINV nstructions.

This pattern relies on a few general changes that will be used throughout
the following patches:
                - A new macro, TARGET_COND_ARITH, which is only true on 8.1-M Mainline
                  and represents the existence of these conditional instructions.
                - A change to the cond exec hook, arm_have_conditional_execution, which
                  now returns false if TARGET_COND_ARITH before reload. This allows for
                  some ifcvt transformations when they would usually be disabled. I've
                  written a rather verbose comment (with the risk of over-explaining)
                  as it's a bit of a confusing change.
                - One new predicate and one new constraint.
                - *thumb2_movcond has been restricted to only match if !TARGET_COND_ARITH,
                  otherwise it triggers undesirable combines.

2020-07-30: Sudakshina Das <sudi.das@arm.com>
            Omar Tahir <omar.tahir@arm.com>

                * config/arm/arm.h (TARGET_COND_ARITH): New macro.
                * config/arm/arm.c (arm_have_conditional_execution): Return false if
                TARGET_COND_ARITH before reload.
                * config/arm/constraints.md: (Z): New constant zero.
                * config/arm/predicates.md(arm_comparison_operation): Returns true if
                comparing CC_REGNUM with constant zero.
                * config/arm/thumb2.md (*thumb2_csinv): New.
                (*thumb2_movcond): Don't match if TARGET_COND_ARITH.

Regression tested on arm-none-eabi.


gcc/testsuite/ChangeLog:

2020-07-30: Sudakshina Das <sudi.das@arm.com>
            Omar Tahir <omar.tahir@arm.com>

                * gcc.target/arm/csinv-1.c: New test.


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dac9a6fb5c4..3a9684cdcd8 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29833,12 +29833,20 @@ arm_frame_pointer_required (void)
   return false;
}
-/* Only thumb1 can't support conditional execution, so return true if
-   the target is not thumb1.  */
static bool
arm_have_conditional_execution (void)
{
-  return !TARGET_THUMB1;
+  bool has_cond_exec, enable_ifcvt_trans;
+
+  /* Only THUMB1 cannot support conditional execution. */
+  has_cond_exec = !TARGET_THUMB1;
+
+  /* When TARGET_COND_ARITH is defined we'd like to turn on some ifcvt
+     transformations before reload. */
+  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
+
+  /* The ifcvt transformations are only turned on if we return false. */
+  return has_cond_exec && !enable_ifcvt_trans;
}
 /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 30e1d6dc994..d67c91796e4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */
 #define TARGET_CRC32                                               (arm_arch_crc)
+/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
+   csinv, etc. */
+#define TARGET_COND_ARITH                (arm_arch8_1m_main)
+
/* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index 011badc9957..048b25ef4a1 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -28,6 +28,7 @@
;; The following normal constraints have been used:
;; in ARM/Thumb-2 state: G, I, j, J, K, L, M
;; in Thumb-1 state: I, J, K, L, M, N, O
+;; in all states: Z
;; 'H' was previously used for FPA.
 ;; The following multi-letter normal constraints have been used:
@@ -479,6 +480,11 @@
  (and (match_code "mem")
       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 1, true)")))
+(define_constraint "Z"
+  "@internal
+   Integer constant zero."
+  (match_test "op == const0_rtx"))
+
(define_memory_constraint "Ux"
  "@internal
   In ARM/Thumb-2 state a valid address and load into CORE regs or only to
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 981eec520ba..2144520829c 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -485,6 +485,18 @@
   (and (match_operand 0 "expandable_comparison_operator")
        (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
+(define_special_predicate "arm_comparison_operation"
+  (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+         ordered,unlt,unle,unge,ungt")
+{
+  if (XEXP (op, 1) != const0_rtx)
+    return false;
+  rtx op0 = XEXP (op, 0);
+  if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+    return false;
+  return maybe_get_arm_condition_code (op) != ARM_NV;
+})
+
(define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 793f6706868..0b00aef7ef7 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -938,6 +938,20 @@
    (set_attr "type" "multiple")]
)
+(define_insn "*thumb2_csinv"
+  [(set (match_operand:SI 0 "arm_general_register_operand" "=r, r")
+  (if_then_else:SI
+    (match_operand 1 "arm_comparison_operation" "")
+    (not:SI (match_operand:SI 2 "arm_general_register_operand" "r, r"))
+    (match_operand:SI 3 "reg_or_zero_operand" "r, Z")))]
+  "TARGET_COND_ARITH"
+  "@
+   csinv\\t%0, %3, %2, %D1
+   csinv\\t%0, zr, %2, %D1"
+  [(set_attr "type" "csel")
+   (set_attr "predicable" "no")]
+)
+
(define_insn "*thumb2_movcond"
   [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts,Ts")
               (if_then_else:SI
@@ -947,7 +961,7 @@
                (match_operand:SI 1 "arm_rhs_operand" "0,TsI,?TsI")
                (match_operand:SI 2 "arm_rhs_operand" "TsI,0,TsI")))
    (clobber (reg:CC CC_REGNUM))]
-  "TARGET_THUMB2"
+  "TARGET_THUMB2 && !TARGET_COND_ARITH"
   "*
   if (GET_CODE (operands[5]) == LT
       && (operands[4] == const0_rtx))
diff --git a/gcc/testsuite/gcc.target/arm/csinv-1.c b/gcc/testsuite/gcc.target/arm/csinv-1.c
new file mode 100644
index 00000000000..6b5383aa913
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/csinv-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-options "-O2 -march=armv8.1-m.main" } */
+
+int
+test_csinv32_condasn1(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*ne" } } */
+  w4 = (w0 == w1) ? ~w2 : w3;
+  return w4;
+}
+
+int
+test_csinv32_condasn2(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*eq" } } */
+  w4 = (w0 == w1) ? w3 : ~w2;
+  return w4;
+}

[-- Attachment #2: csel_2.patch --]
[-- Type: application/octet-stream, Size: 5009 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dac9a6fb5c4..3a9684cdcd8 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29833,12 +29833,20 @@ arm_frame_pointer_required (void)
   return false;
 }
 
-/* Only thumb1 can't support conditional execution, so return true if
-   the target is not thumb1.  */
 static bool
 arm_have_conditional_execution (void)
 {
-  return !TARGET_THUMB1;
+  bool has_cond_exec, enable_ifcvt_trans;
+
+  /* Only THUMB1 cannot support conditional execution. */
+  has_cond_exec = !TARGET_THUMB1;
+
+  /* When TARGET_COND_ARITH is defined we'd like to turn on some ifcvt
+     transformations before reload. */
+  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
+
+  /* The ifcvt transformations are only turned on if we return false. */
+  return has_cond_exec && !enable_ifcvt_trans;
 }
 
 /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 30e1d6dc994..d67c91796e4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */
 
 #define TARGET_CRC32			(arm_arch_crc)
 
+/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
+   csinv, etc. */
+#define TARGET_COND_ARITH		(arm_arch8_1m_main)
+
 /* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index 011badc9957..048b25ef4a1 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -28,6 +28,7 @@
 ;; The following normal constraints have been used:
 ;; in ARM/Thumb-2 state: G, I, j, J, K, L, M
 ;; in Thumb-1 state: I, J, K, L, M, N, O
+;; in all states: Z
 ;; 'H' was previously used for FPA.
 
 ;; The following multi-letter normal constraints have been used:
@@ -479,6 +480,11 @@
  (and (match_code "mem")
       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 1, true)")))
 
+(define_constraint "Z"
+  "@internal
+   Integer constant zero."
+  (match_test "op == const0_rtx"))
+
 (define_memory_constraint "Ux"
  "@internal
   In ARM/Thumb-2 state a valid address and load into CORE regs or only to
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 981eec520ba..2144520829c 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -485,6 +485,18 @@
   (and (match_operand 0 "expandable_comparison_operator")
        (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
 
+(define_special_predicate "arm_comparison_operation"
+  (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+         ordered,unlt,unle,unge,ungt")
+{
+  if (XEXP (op, 1) != const0_rtx)
+    return false;
+  rtx op0 = XEXP (op, 0);
+  if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+    return false;
+  return maybe_get_arm_condition_code (op) != ARM_NV;
+})
+
 (define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
 
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 793f6706868..0b00aef7ef7 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -938,6 +938,20 @@
    (set_attr "type" "multiple")]
 )
 
+(define_insn "*thumb2_csinv"
+  [(set (match_operand:SI 0 "arm_general_register_operand" "=r, r")
+  (if_then_else:SI
+    (match_operand 1 "arm_comparison_operation" "")
+    (not:SI (match_operand:SI 2 "arm_general_register_operand" "r, r"))
+    (match_operand:SI 3 "reg_or_zero_operand" "r, Z")))]
+  "TARGET_COND_ARITH"
+  "@
+   csinv\\t%0, %3, %2, %D1
+   csinv\\t%0, zr, %2, %D1"
+  [(set_attr "type" "csel")
+   (set_attr "predicable" "no")]
+)
+
 (define_insn "*thumb2_movcond"
   [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts,Ts")
 	(if_then_else:SI
@@ -947,7 +961,7 @@
 	 (match_operand:SI 1 "arm_rhs_operand" "0,TsI,?TsI")
 	 (match_operand:SI 2 "arm_rhs_operand" "TsI,0,TsI")))
    (clobber (reg:CC CC_REGNUM))]
-  "TARGET_THUMB2"
+  "TARGET_THUMB2 && !TARGET_COND_ARITH"
   "*
   if (GET_CODE (operands[5]) == LT
       && (operands[4] == const0_rtx))
diff --git a/gcc/testsuite/gcc.target/arm/csinv-1.c b/gcc/testsuite/gcc.target/arm/csinv-1.c
new file mode 100644
index 00000000000..6b5383aa913
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/csinv-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-options "-O2 -march=armv8.1-m.main" } */
+
+int
+test_csinv32_condasn1(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*ne" } } */
+  w4 = (w0 == w1) ? ~w2 : w3;
+  return w4;
+}
+
+int
+test_csinv32_condasn2(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*eq" } } */
+  w4 = (w0 == w1) ? w3 : ~w2;
+  return w4;
+}

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

* RE: [PATCH 2/5][Arm] New pattern for CSINV instructions
  2020-08-04 16:11 [PATCH 2/5][Arm] New pattern for CSINV instructions Omar Tahir
@ 2020-08-05  8:45 ` Kyrylo Tkachov
  2020-08-05 11:41   ` Omar Tahir
  0 siblings, 1 reply; 4+ messages in thread
From: Kyrylo Tkachov @ 2020-08-05  8:45 UTC (permalink / raw)
  To: Omar Tahir, nickc, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

Hi Omar,

From: Omar Tahir <Omar.Tahir@arm.com> 
Sent: 04 August 2020 17:12
To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; nickc@redhat.com; Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>; Richard Earnshaw <Richard.Earnshaw@arm.com>; gcc-patches@gcc.gnu.org
Subject: [PATCH 2/5][Arm] New pattern for CSINV instructions

This patch adds a new pattern, *thumb2_csinv, for generating CSINV nstructions.

This pattern relies on a few general changes that will be used throughout
the following patches:
	- A new macro, TARGET_COND_ARITH, which is only true on 8.1-M Mainline
	  and represents the existence of these conditional instructions.
	- A change to the cond exec hook, arm_have_conditional_execution, which
	  now returns false if TARGET_COND_ARITH before reload. This allows for
	  some ifcvt transformations when they would usually be disabled. I've
	  written a rather verbose comment (with the risk of over-explaining)
	  as it's a bit of a confusing change.
	- One new predicate and one new constraint.
	- *thumb2_movcond has been restricted to only match if !TARGET_COND_ARITH,
	  otherwise it triggers undesirable combines.

2020-07-30: Sudakshina Das <sudi.das@arm.com>
            Omar Tahir <omar.tahir@arm.com>


No ":" in the ChangeLog and two spaces between the 3 fields.


	* config/arm/arm.h (TARGET_COND_ARITH): New macro.
	* config/arm/arm.c (arm_have_conditional_execution): Return false if
	TARGET_COND_ARITH before reload.
	* config/arm/constraints.md: (Z): New constant zero.
	* config/arm/predicates.md(arm_comparison_operation): Returns true if
	comparing CC_REGNUM with constant zero.
	* config/arm/thumb2.md (*thumb2_csinv): New.
	(*thumb2_movcond): Don't match if TARGET_COND_ARITH.

Regression tested on arm-none-eabi.


gcc/testsuite/ChangeLog:

2020-07-30: Sudakshina Das <sudi.das@arm.com>
            Omar Tahir <omar.tahir@arm.com>

	* gcc.target/arm/csinv-1.c: New test.


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dac9a6fb5c4..3a9684cdcd8 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29833,12 +29833,20 @@ arm_frame_pointer_required (void)
   return false;
}

-/* Only thumb1 can't support conditional execution, so return true if
-   the target is not thumb1.  */
static bool


Functions should have comments in GCC. Can you please write something describing the new logic of the function.

arm_have_conditional_execution (void)
{
-  return !TARGET_THUMB1;
+  bool has_cond_exec, enable_ifcvt_trans;
+
+  /* Only THUMB1 cannot support conditional execution. */
+  has_cond_exec = !TARGET_THUMB1;
+
+  /* When TARGET_COND_ARITH is defined we'd like to turn on some ifcvt
+     transformations before reload. */
+  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
+
+  /* The ifcvt transformations are only turned on if we return false. */
+  return has_cond_exec && !enable_ifcvt_trans;

I don't think that comment is very useful. Perhaps "Enable ifcvt transformations only if..."

}

 /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 30e1d6dc994..d67c91796e4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */

 #define TARGET_CRC32			(arm_arch_crc)

+/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
+   csinv, etc. */
+#define TARGET_COND_ARITH		(arm_arch8_1m_main)
+
/* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
diff --git a/gcc/config/arm/constraints.md b/gcc/config/arm/constraints.md
index 011badc9957..048b25ef4a1 100644
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -28,6 +28,7 @@
;; The following normal constraints have been used:
;; in ARM/Thumb-2 state: G, I, j, J, K, L, M
;; in Thumb-1 state: I, J, K, L, M, N, O
+;; in all states: Z
;; 'H' was previously used for FPA.

 ;; The following multi-letter normal constraints have been used:
@@ -479,6 +480,11 @@
  (and (match_code "mem")
       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 1, true)")))

+(define_constraint "Z"
+  "@internal
+   Integer constant zero."
+  (match_test "op == const0_rtx"))


We're usually wary of adding more constraints unless necessary as it gets complicated to read patterns quickly (especially once we get into multi-letter constraints).
I think you can reuse the existing "Pz" constraint for your purposes.


Ok with those changes.
If you'd like to commit it yourself please apply for write access at https://sourceware.org/cgi-bin/pdw/ps_form.cgi listing my email address from MAINTAINERS as the approver.

Thanks,
Kyrill



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

* RE: [PATCH 2/5][Arm] New pattern for CSINV instructions
  2020-08-05  8:45 ` Kyrylo Tkachov
@ 2020-08-05 11:41   ` Omar Tahir
  2020-08-06  9:30     ` Kyrylo Tkachov
  0 siblings, 1 reply; 4+ messages in thread
From: Omar Tahir @ 2020-08-05 11:41 UTC (permalink / raw)
  To: Kyrylo Tkachov, nickc, Ramana Radhakrishnan, Richard Earnshaw,
	gcc-patches

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

Hi Kyrill,

> -/* Only thumb1 can't support conditional execution, so return true if
> -   the target is not thumb1.  */
> static bool
> 
> 
> Functions should have comments in GCC. Can you please write something describing the new logic of the function.
> 
> arm_have_conditional_execution (void)
> {
> -  return !TARGET_THUMB1;
> +  bool has_cond_exec, enable_ifcvt_trans;
> +
> +  /* Only THUMB1 cannot support conditional execution. */
> +  has_cond_exec = !TARGET_THUMB1;
> +
> +  /* When TARGET_COND_ARITH is defined we'd like to turn on some ifcvt
> +     transformations before reload. */
> +  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
> +
> +  /* The ifcvt transformations are only turned on if we return false. */
> +  return has_cond_exec && !enable_ifcvt_trans;
> 
> I don't think that comment is very useful. Perhaps "Enable ifcvt transformations only if..."
> 
> }

Fixed, let me know if the new comments are a bit clearer now.

> +(define_constraint "Z"
> +  "@internal
> +   Integer constant zero."
> +  (match_test "op == const0_rtx"))
> 
> 
> We're usually wary of adding more constraints unless necessary as it gets complicated to read patterns quickly (especially once we get into multi-letter constraints).
> I think you can reuse the existing "Pz" constraint for your purposes.

Yes Pz works, I'll replace Z with Pz in the other patches as well. In patch 5 I introduce UM (-1) and U1 (1), I don't think there's any existing combination of constraints that can be used instead.

> 
> Ok with those changes.
> If you'd like to commit it yourself please apply for write access at https://sourceware.org/cgi-bin/pdw/ps_form.cgi listing my email address from MAINTAINERS as the approver.

Excellent, thanks. If the other three patches are okay I'll commit them as well?

Thanks,
Omar

---

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dac9a6fb5c4..e1bb2db9c8a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29833,12 +29833,23 @@ arm_frame_pointer_required (void)
   return false;
 }
 
-/* Only thumb1 can't support conditional execution, so return true if
-   the target is not thumb1.  */
+/* Implement the TARGET_HAVE_CONDITIONAL_EXECUTION hook.
+   All modes except THUMB1 have conditional execution.
+   If we have conditional arithmetic, return false before reload to
+   enable some ifcvt transformations. */
 static bool
 arm_have_conditional_execution (void)
 {
-  return !TARGET_THUMB1;
+  bool has_cond_exec, enable_ifcvt_trans;
+
+  /* Only THUMB1 cannot support conditional execution. */
+  has_cond_exec = !TARGET_THUMB1;
+
+  /* Enable ifcvt transformations if we have conditional arithmetic, but only
+     before reload. */
+  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
+
+  return has_cond_exec && !enable_ifcvt_trans;
 }
 
 /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 30e1d6dc994..d67c91796e4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */
 
 #define TARGET_CRC32			(arm_arch_crc)
 
+/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
+   csinv, etc. */
+#define TARGET_COND_ARITH		(arm_arch8_1m_main)
+
 /* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 981eec520ba..2144520829c 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -485,6 +485,18 @@
   (and (match_operand 0 "expandable_comparison_operator")
        (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
 
+(define_special_predicate "arm_comparison_operation"
+  (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+         ordered,unlt,unle,unge,ungt")
+{
+  if (XEXP (op, 1) != const0_rtx)
+    return false;
+  rtx op0 = XEXP (op, 0);
+  if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+    return false;
+  return maybe_get_arm_condition_code (op) != ARM_NV;
+})
+
 (define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
 
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 793f6706868..ecc903970db 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -938,6 +938,20 @@
    (set_attr "type" "multiple")]
 )
 
+(define_insn "*thumb2_csinv"
+  [(set (match_operand:SI 0 "arm_general_register_operand" "=r, r")
+  (if_then_else:SI
+    (match_operand 1 "arm_comparison_operation" "")
+    (not:SI (match_operand:SI 2 "arm_general_register_operand" "r, r"))
+    (match_operand:SI 3 "reg_or_zero_operand" "r, Pz")))]
+  "TARGET_COND_ARITH"
+  "@
+   csinv\\t%0, %3, %2, %D1
+   csinv\\t%0, zr, %2, %D1"
+  [(set_attr "type" "csel")
+   (set_attr "predicable" "no")]
+)
+
 (define_insn "*thumb2_movcond"
   [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts,Ts")
 	(if_then_else:SI
@@ -947,7 +961,7 @@
 	 (match_operand:SI 1 "arm_rhs_operand" "0,TsI,?TsI")
 	 (match_operand:SI 2 "arm_rhs_operand" "TsI,0,TsI")))
    (clobber (reg:CC CC_REGNUM))]
-  "TARGET_THUMB2"
+  "TARGET_THUMB2 && !TARGET_COND_ARITH"
   "*
   if (GET_CODE (operands[5]) == LT
       && (operands[4] == const0_rtx))
diff --git a/gcc/testsuite/gcc.target/arm/csinv-1.c b/gcc/testsuite/gcc.target/arm/csinv-1.c
new file mode 100644
index 00000000000..6b5383aa913
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/csinv-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-options "-O2 -march=armv8.1-m.main" } */
+
+int
+test_csinv32_condasn1(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*ne" } } */
+  w4 = (w0 == w1) ? ~w2 : w3;
+  return w4;
+}
+
+int
+test_csinv32_condasn2(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*eq" } } */
+  w4 = (w0 == w1) ? w3 : ~w2;
+  return w4;
+}

[-- Attachment #2: csel_2.patch --]
[-- Type: application/octet-stream, Size: 4331 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index dac9a6fb5c4..e1bb2db9c8a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29833,12 +29833,23 @@ arm_frame_pointer_required (void)
   return false;
 }
 
-/* Only thumb1 can't support conditional execution, so return true if
-   the target is not thumb1.  */
+/* Implement the TARGET_HAVE_CONDITIONAL_EXECUTION hook.
+   All modes except THUMB1 have conditional execution.
+   If we have conditional arithmetic, return false before reload to
+   enable some ifcvt transformations. */
 static bool
 arm_have_conditional_execution (void)
 {
-  return !TARGET_THUMB1;
+  bool has_cond_exec, enable_ifcvt_trans;
+
+  /* Only THUMB1 cannot support conditional execution. */
+  has_cond_exec = !TARGET_THUMB1;
+
+  /* Enable ifcvt transformations if we have conditional arithmetic, but only
+     before reload. */
+  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
+
+  return has_cond_exec && !enable_ifcvt_trans;
 }
 
 /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 30e1d6dc994..d67c91796e4 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */
 
 #define TARGET_CRC32			(arm_arch_crc)
 
+/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
+   csinv, etc. */
+#define TARGET_COND_ARITH		(arm_arch8_1m_main)
+
 /* The following two macros concern the ability to execute coprocessor
    instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are currently
    only ever tested when we know we are generating for VFP hardware; we need
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index 981eec520ba..2144520829c 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -485,6 +485,18 @@
   (and (match_operand 0 "expandable_comparison_operator")
        (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
 
+(define_special_predicate "arm_comparison_operation"
+  (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
+         ordered,unlt,unle,unge,ungt")
+{
+  if (XEXP (op, 1) != const0_rtx)
+    return false;
+  rtx op0 = XEXP (op, 0);
+  if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
+    return false;
+  return maybe_get_arm_condition_code (op) != ARM_NV;
+})
+
 (define_special_predicate "lt_ge_comparison_operator"
   (match_code "lt,ge"))
 
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 793f6706868..ecc903970db 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -938,6 +938,20 @@
    (set_attr "type" "multiple")]
 )
 
+(define_insn "*thumb2_csinv"
+  [(set (match_operand:SI 0 "arm_general_register_operand" "=r, r")
+  (if_then_else:SI
+    (match_operand 1 "arm_comparison_operation" "")
+    (not:SI (match_operand:SI 2 "arm_general_register_operand" "r, r"))
+    (match_operand:SI 3 "reg_or_zero_operand" "r, Pz")))]
+  "TARGET_COND_ARITH"
+  "@
+   csinv\\t%0, %3, %2, %D1
+   csinv\\t%0, zr, %2, %D1"
+  [(set_attr "type" "csel")
+   (set_attr "predicable" "no")]
+)
+
 (define_insn "*thumb2_movcond"
   [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts,Ts")
 	(if_then_else:SI
@@ -947,7 +961,7 @@
 	 (match_operand:SI 1 "arm_rhs_operand" "0,TsI,?TsI")
 	 (match_operand:SI 2 "arm_rhs_operand" "TsI,0,TsI")))
    (clobber (reg:CC CC_REGNUM))]
-  "TARGET_THUMB2"
+  "TARGET_THUMB2 && !TARGET_COND_ARITH"
   "*
   if (GET_CODE (operands[5]) == LT
       && (operands[4] == const0_rtx))
diff --git a/gcc/testsuite/gcc.target/arm/csinv-1.c b/gcc/testsuite/gcc.target/arm/csinv-1.c
new file mode 100644
index 00000000000..6b5383aa913
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/csinv-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
+/* { dg-options "-O2 -march=armv8.1-m.main" } */
+
+int
+test_csinv32_condasn1(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*ne" } } */
+  w4 = (w0 == w1) ? ~w2 : w3;
+  return w4;
+}
+
+int
+test_csinv32_condasn2(int w0, int w1, int w2, int w3)
+{
+  int w4;
+
+  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*eq" } } */
+  w4 = (w0 == w1) ? w3 : ~w2;
+  return w4;
+}

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

* RE: [PATCH 2/5][Arm] New pattern for CSINV instructions
  2020-08-05 11:41   ` Omar Tahir
@ 2020-08-06  9:30     ` Kyrylo Tkachov
  0 siblings, 0 replies; 4+ messages in thread
From: Kyrylo Tkachov @ 2020-08-06  9:30 UTC (permalink / raw)
  To: Omar Tahir, nickc, Ramana Radhakrishnan, Richard Earnshaw, gcc-patches

Hi Omar,

> -----Original Message-----
> From: Omar Tahir <Omar.Tahir@arm.com>
> Sent: 05 August 2020 12:42
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; nickc@redhat.com;
> Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>; Richard
> Earnshaw <Richard.Earnshaw@arm.com>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH 2/5][Arm] New pattern for CSINV instructions
> 
> Hi Kyrill,
> 
> > -/* Only thumb1 can't support conditional execution, so return true if
> > -   the target is not thumb1.  */
> > static bool
> >
> >
> > Functions should have comments in GCC. Can you please write something
> describing the new logic of the function.
> >
> > arm_have_conditional_execution (void)
> > {
> > -  return !TARGET_THUMB1;
> > +  bool has_cond_exec, enable_ifcvt_trans;
> > +
> > +  /* Only THUMB1 cannot support conditional execution. */
> > +  has_cond_exec = !TARGET_THUMB1;
> > +
> > +  /* When TARGET_COND_ARITH is defined we'd like to turn on some ifcvt
> > +     transformations before reload. */
> > +  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
> > +
> > +  /* The ifcvt transformations are only turned on if we return false. */
> > +  return has_cond_exec && !enable_ifcvt_trans;
> >
> > I don't think that comment is very useful. Perhaps "Enable ifcvt
> transformations only if..."
> >
> > }
> 
> Fixed, let me know if the new comments are a bit clearer now.
> 
> > +(define_constraint "Z"
> > +  "@internal
> > +   Integer constant zero."
> > +  (match_test "op == const0_rtx"))
> >
> >
> > We're usually wary of adding more constraints unless necessary as it gets
> complicated to read patterns quickly (especially once we get into multi-letter
> constraints).
> > I think you can reuse the existing "Pz" constraint for your purposes.
> 
> Yes Pz works, I'll replace Z with Pz in the other patches as well. In patch 5 I
> introduce UM (-1) and U1 (1), I don't think there's any existing combination
> of constraints that can be used instead.

Great!

> 
> >
> > Ok with those changes.
> > If you'd like to commit it yourself please apply for write access at
> https://sourceware.org/cgi-bin/pdw/ps_form.cgi listing my email address
> from MAINTAINERS as the approver.
> 
> Excellent, thanks. If the other three patches are okay I'll commit them as well?

Please wait for review before committing them, but once they're ok'ed feel free to push them (please make sure proper testing is done so that trunk is not left in a broken state).

Thanks,
Kyrill

> 
> Thanks,
> Omar
> 
> ---
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index dac9a6fb5c4..e1bb2db9c8a 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -29833,12 +29833,23 @@ arm_frame_pointer_required (void)
>    return false;
>  }
> 
> -/* Only thumb1 can't support conditional execution, so return true if
> -   the target is not thumb1.  */
> +/* Implement the TARGET_HAVE_CONDITIONAL_EXECUTION hook.
> +   All modes except THUMB1 have conditional execution.
> +   If we have conditional arithmetic, return false before reload to
> +   enable some ifcvt transformations. */
>  static bool
>  arm_have_conditional_execution (void)
>  {
> -  return !TARGET_THUMB1;
> +  bool has_cond_exec, enable_ifcvt_trans;
> +
> +  /* Only THUMB1 cannot support conditional execution. */
> +  has_cond_exec = !TARGET_THUMB1;
> +
> +  /* Enable ifcvt transformations if we have conditional arithmetic, but only
> +     before reload. */
> +  enable_ifcvt_trans = TARGET_COND_ARITH && !reload_completed;
> +
> +  return has_cond_exec && !enable_ifcvt_trans;
>  }
> 
>  /* The AAPCS sets the maximum alignment of a vector to 64 bits.  */
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index 30e1d6dc994..d67c91796e4 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -177,6 +177,10 @@ emission of floating point pcs attributes.  */
> 
>  #define TARGET_CRC32			(arm_arch_crc)
> 
> +/* Thumb-2 but also has some conditional arithmetic instructions like csinc,
> +   csinv, etc. */
> +#define TARGET_COND_ARITH		(arm_arch8_1m_main)
> +
>  /* The following two macros concern the ability to execute coprocessor
>     instructions for VFPv3 or NEON.  TARGET_VFP3/TARGET_VFPD32 are
> currently
>     only ever tested when we know we are generating for VFP hardware; we
> need
> diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
> index 981eec520ba..2144520829c 100644
> --- a/gcc/config/arm/predicates.md
> +++ b/gcc/config/arm/predicates.md
> @@ -485,6 +485,18 @@
>    (and (match_operand 0 "expandable_comparison_operator")
>         (match_test "maybe_get_arm_condition_code (op) != ARM_NV")))
> 
> +(define_special_predicate "arm_comparison_operation"
> +  (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,
> +         ordered,unlt,unle,unge,ungt")
> +{
> +  if (XEXP (op, 1) != const0_rtx)
> +    return false;
> +  rtx op0 = XEXP (op, 0);
> +  if (!REG_P (op0) || REGNO (op0) != CC_REGNUM)
> +    return false;
> +  return maybe_get_arm_condition_code (op) != ARM_NV;
> +})
> +
>  (define_special_predicate "lt_ge_comparison_operator"
>    (match_code "lt,ge"))
> 
> diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
> index 793f6706868..ecc903970db 100644
> --- a/gcc/config/arm/thumb2.md
> +++ b/gcc/config/arm/thumb2.md
> @@ -938,6 +938,20 @@
>     (set_attr "type" "multiple")]
>  )
> 
> +(define_insn "*thumb2_csinv"
> +  [(set (match_operand:SI 0 "arm_general_register_operand" "=r, r")
> +  (if_then_else:SI
> +    (match_operand 1 "arm_comparison_operation" "")
> +    (not:SI (match_operand:SI 2 "arm_general_register_operand" "r, r"))
> +    (match_operand:SI 3 "reg_or_zero_operand" "r, Pz")))]
> +  "TARGET_COND_ARITH"
> +  "@
> +   csinv\\t%0, %3, %2, %D1
> +   csinv\\t%0, zr, %2, %D1"
> +  [(set_attr "type" "csel")
> +   (set_attr "predicable" "no")]
> +)
> +
>  (define_insn "*thumb2_movcond"
>    [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts,Ts")
>  	(if_then_else:SI
> @@ -947,7 +961,7 @@
>  	 (match_operand:SI 1 "arm_rhs_operand" "0,TsI,?TsI")
>  	 (match_operand:SI 2 "arm_rhs_operand" "TsI,0,TsI")))
>     (clobber (reg:CC CC_REGNUM))]
> -  "TARGET_THUMB2"
> +  "TARGET_THUMB2 && !TARGET_COND_ARITH"
>    "*
>    if (GET_CODE (operands[5]) == LT
>        && (operands[4] == const0_rtx))
> diff --git a/gcc/testsuite/gcc.target/arm/csinv-1.c
> b/gcc/testsuite/gcc.target/arm/csinv-1.c
> new file mode 100644
> index 00000000000..6b5383aa913
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/csinv-1.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_ok } */
> +/* { dg-options "-O2 -march=armv8.1-m.main" } */
> +
> +int
> +test_csinv32_condasn1(int w0, int w1, int w2, int w3)
> +{
> +  int w4;
> +
> +  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*ne" } } */
> +  w4 = (w0 == w1) ? ~w2 : w3;
> +  return w4;
> +}
> +
> +int
> +test_csinv32_condasn2(int w0, int w1, int w2, int w3)
> +{
> +  int w4;
> +
> +  /* { dg-final { scan-assembler "csinv\tr\[0-9\]*.*eq" } } */
> +  w4 = (w0 == w1) ? w3 : ~w2;
> +  return w4;
> +}

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

end of thread, other threads:[~2020-08-06  9:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-04 16:11 [PATCH 2/5][Arm] New pattern for CSINV instructions Omar Tahir
2020-08-05  8:45 ` Kyrylo Tkachov
2020-08-05 11:41   ` Omar Tahir
2020-08-06  9:30     ` Kyrylo Tkachov

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).