aarch64: Use canonicalize_comparison in ccmp expansion [PR117346]

Message ID 20241029191549.1217135-1-quic_apinski@quicinc.com
State Committed
Commit 3d8cd34a450e9ffe2b2ac8a0c8eb33fd5d613483
Headers
Series aarch64: Use canonicalize_comparison in ccmp expansion [PR117346] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Patch failed to apply

Commit Message

Andrew Pinski Oct. 29, 2024, 7:15 p.m. UTC
  While testing the patch for PR 85605 on aarch64, it was noticed that
imm_choice_comparison.c test failed. This was because canonicalize_comparison
was not being called in the ccmp case. This can be noticed without the patch
for PR 85605 as evidence of the new testcase.

Bootstrapped and tested on aarch64-linux-gnu.

	PR target/117346

gcc/ChangeLog:

	* config/aarch64/aarch64.cc (aarch64_gen_ccmp_first): Call
	canonicalize_comparison before figuring out the cmp_mode/cc_mode.
	(aarch64_gen_ccmp_next): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/imm_choice_comparison-1.c: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/config/aarch64/aarch64.cc                 |  6 +++
 .../aarch64/imm_choice_comparison-1.c         | 42 +++++++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
  

Comments

Richard Sandiford Oct. 29, 2024, 10:31 p.m. UTC | #1
Andrew Pinski <quic_apinski@quicinc.com> writes:
> While testing the patch for PR 85605 on aarch64, it was noticed that
> imm_choice_comparison.c test failed. This was because canonicalize_comparison
> was not being called in the ccmp case. This can be noticed without the patch
> for PR 85605 as evidence of the new testcase.
>
> Bootstrapped and tested on aarch64-linux-gnu.
>
> 	PR target/117346
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64.cc (aarch64_gen_ccmp_first): Call
> 	canonicalize_comparison before figuring out the cmp_mode/cc_mode.
> 	(aarch64_gen_ccmp_next): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/imm_choice_comparison-1.c: New test.

OK, thanks.

Richard

> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/config/aarch64/aarch64.cc                 |  6 +++
>  .../aarch64/imm_choice_comparison-1.c         | 42 +++++++++++++++++++
>  2 files changed, 48 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index a6cc00e74ab..cbb7ef13315 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -27353,6 +27353,9 @@ aarch64_gen_ccmp_first (rtx_insn **prep_seq, rtx_insn **gen_seq,
>    if (op_mode == VOIDmode)
>      op_mode = GET_MODE (op1);
>  
> +  if (CONST_SCALAR_INT_P (op1))
> +    canonicalize_comparison (op_mode, &code, &op1);
> +
>    switch (op_mode)
>      {
>      case E_QImode:
> @@ -27429,6 +27432,9 @@ aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev,
>    if (op_mode == VOIDmode)
>      op_mode = GET_MODE (op1);
>  
> +  if (CONST_SCALAR_INT_P (op1))
> +    canonicalize_comparison (op_mode, &cmp_code, &op1);
> +
>    switch (op_mode)
>      {
>      case E_QImode:
> diff --git a/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c b/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
> new file mode 100644
> index 00000000000..2afebe1a349
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/* PR target/117346 */
> +/* Make sure going through ccmp uses similar to non ccmp-case. */
> +/* This is similar to imm_choice_comparison.c's check except to force
> +   the use of ccmp by reording the comparison and putting the cast before. */
> +
> +/*
> +** check:
> +**	...
> +**	mov	w[0-9]+, -16777217
> +**	...
> +*/
> +
> +int
> +check (int x, int y)
> +{
> +  unsigned xu = x;
> +  if (xu > 0xfefffffe && x > y)
> +    return 100;
> +
> +  return x;
> +}
> +
> +/*
> +** check1:
> +**	...
> +**	mov	w[0-9]+, -16777217
> +**	...
> +*/
> +
> +int
> +check1 (int x, int y)
> +{
> +  unsigned xu = x;
> +  if (x > y && xu > 0xfefffffe)
> +    return 100;
> +
> +  return x;
> +}
  

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index a6cc00e74ab..cbb7ef13315 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -27353,6 +27353,9 @@  aarch64_gen_ccmp_first (rtx_insn **prep_seq, rtx_insn **gen_seq,
   if (op_mode == VOIDmode)
     op_mode = GET_MODE (op1);
 
+  if (CONST_SCALAR_INT_P (op1))
+    canonicalize_comparison (op_mode, &code, &op1);
+
   switch (op_mode)
     {
     case E_QImode:
@@ -27429,6 +27432,9 @@  aarch64_gen_ccmp_next (rtx_insn **prep_seq, rtx_insn **gen_seq, rtx prev,
   if (op_mode == VOIDmode)
     op_mode = GET_MODE (op1);
 
+  if (CONST_SCALAR_INT_P (op1))
+    canonicalize_comparison (op_mode, &cmp_code, &op1);
+
   switch (op_mode)
     {
     case E_QImode:
diff --git a/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c b/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
new file mode 100644
index 00000000000..2afebe1a349
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/imm_choice_comparison-1.c
@@ -0,0 +1,42 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* PR target/117346 */
+/* Make sure going through ccmp uses similar to non ccmp-case. */
+/* This is similar to imm_choice_comparison.c's check except to force
+   the use of ccmp by reording the comparison and putting the cast before. */
+
+/*
+** check:
+**	...
+**	mov	w[0-9]+, -16777217
+**	...
+*/
+
+int
+check (int x, int y)
+{
+  unsigned xu = x;
+  if (xu > 0xfefffffe && x > y)
+    return 100;
+
+  return x;
+}
+
+/*
+** check1:
+**	...
+**	mov	w[0-9]+, -16777217
+**	...
+*/
+
+int
+check1 (int x, int y)
+{
+  unsigned xu = x;
+  if (x > y && xu > 0xfefffffe)
+    return 100;
+
+  return x;
+}