Hi Omar,
From: Omar Tahir <omar.ta...@arm.com>
Sent: 04 August 2020 17:12
To: Kyrylo Tkachov <kyrylo.tkac...@arm.com>; ni...@redhat.com; Ramana
Radhakrishnan <ramana.radhakrish...@arm.com>; Richard Earnshaw
<richard.earns...@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....@arm.com>
Omar Tahir <omar.ta...@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....@arm.com>
Omar Tahir <omar.ta...@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
