[PATCH 2/5][Arm] New pattern for CSINV instructions

Kyrylo Tkachov Kyrylo.Tkachov@arm.com
Wed Aug 5 08:45:33 GMT 2020

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.


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.


More information about the Gcc-patches mailing list