public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9]
@ 2013-07-29 14:06 Kyrylo Tkachov
  2013-07-30 10:19 ` Richard Earnshaw
  0 siblings, 1 reply; 4+ messages in thread
From: Kyrylo Tkachov @ 2013-07-29 14:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: Marcus Shawcroft

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

Hi all,

Now that combine emits the canonical form for (eq (neg x) (y)) instead of (eq
(x) (neg y)), this patch fixes up the corresponding pattern in aarch64 to
match that. This enables combine to properly generate the cmn instruction
where appropriate.

Tested aarch64-none-elf on model.

Ok for trunk?

Thanks,
Kyrill

2013-07-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

	* config/aarch64/aarch64.md (compare_neg<mode>): Match canonical RTL
form.

[-- Attachment #2: aarch64-cmn.patch --]
[-- Type: application/octet-stream, Size: 555 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 5d64228..c2faf6a 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -1459,8 +1459,8 @@
 (define_insn "*compare_neg<mode>"
   [(set (reg:CC CC_REGNUM)
 	(compare:CC
-	 (match_operand:GPI 0 "register_operand" "r")
-	 (neg:GPI (match_operand:GPI 1 "register_operand" "r"))))]
+	 (neg:GPI (match_operand:GPI 0 "register_operand" "r"))
+	 (match_operand:GPI 1 "register_operand" "r")))]
   ""
   "cmn\\t%<w>0, %<w>1"
   [(set_attr "v8type" "alus")

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

* Re: [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9]
  2013-07-29 14:06 [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9] Kyrylo Tkachov
@ 2013-07-30 10:19 ` Richard Earnshaw
  2013-09-09 12:53   ` Kyrylo Tkachov
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Earnshaw @ 2013-07-30 10:19 UTC (permalink / raw)
  To: Kyrylo Tkachov; +Cc: gcc-patches, Marcus Shawcroft

On 29/07/13 14:58, Kyrylo Tkachov wrote:
> Hi all,
>
> Now that combine emits the canonical form for (eq (neg x) (y)) instead of (eq
> (x) (neg y)), this patch fixes up the corresponding pattern in aarch64 to
> match that. This enables combine to properly generate the cmn instruction
> where appropriate.
>
> Tested aarch64-none-elf on model.
>
> Ok for trunk?
>

No, this is wrong for inequalities, since in reality it's the second 
operand that's inverted.

You'll need to use CC_SWP mode and then arrange for this to be correctly 
picked by select_cc_mode.

R.

> Thanks,
> Kyrill
>
> 2013-07-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
> 	* config/aarch64/aarch64.md (compare_neg<mode>): Match canonical RTL
> form.=
>
>
> aarch64-cmn.patch
>
>
> N\x18¬n‡r¥ªíÂ)emçhÂyhiם¢w^™©Ý
>


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

* RE: [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9]
  2013-07-30 10:19 ` Richard Earnshaw
@ 2013-09-09 12:53   ` Kyrylo Tkachov
  2013-09-09 14:04     ` Richard Earnshaw
  0 siblings, 1 reply; 4+ messages in thread
From: Kyrylo Tkachov @ 2013-09-09 12:53 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: gcc-patches, Marcus Shawcroft

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

Hi Richard,

> On 29/07/13 14:58, Kyrylo Tkachov wrote:
> > Hi all,
> >
> > Now that combine emits the canonical form for (eq (neg x) (y)) instead
> of (eq
> > (x) (neg y)), this patch fixes up the corresponding pattern in aarch64
> to
> > match that. This enables combine to properly generate the cmn
> instruction
> > where appropriate.
> >
> > Tested aarch64-none-elf on model.
> >
> > Ok for trunk?
> >
> 
> No, this is wrong for inequalities, since in reality it's the second
> operand that's inverted.
> 
> You'll need to use CC_SWP mode and then arrange for this to be correctly
> picked by select_cc_mode.
> 

How's this?

aarch64_select_cc_mode is updated to return CC_SWPmode for these comparisons and
the MD pattern is updated accordingly.

Tested aarch64-none-elf on a model.

Is this ok?

Thanks,
Kyrill

2013-09-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

	* config/aarch64/aarch64.c (aarch64_select_cc_mode): Return CC_SWP for
	comparison with negated operand.
	* config/aarch64/aarch64.md (compare_neg<mode>): Match canonical RTL form.
	
2013-09-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

	* gcc.target/aarch64/cmn-neg.c: New test.


[-- Attachment #2: aarch64-cmn.patch --]
[-- Type: application/octet-stream, Size: 2468 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index aed035a..de1107d 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3313,14 +3313,15 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
 	  || GET_CODE (x) == NEG))
     return CC_NZmode;
 
-  /* A compare with a shifted operand.  Because of canonicalization,
+  /* A compare with a shifted or negated operand.  Because of canonicalization,
      the comparison will have to be swapped when we emit the assembly
      code.  */
   if ((GET_MODE (x) == SImode || GET_MODE (x) == DImode)
       && (GET_CODE (y) == REG || GET_CODE (y) == SUBREG)
       && (GET_CODE (x) == ASHIFT || GET_CODE (x) == ASHIFTRT
 	  || GET_CODE (x) == LSHIFTRT
-	  || GET_CODE (x) == ZERO_EXTEND || GET_CODE (x) == SIGN_EXTEND))
+	  || GET_CODE (x) == ZERO_EXTEND || GET_CODE (x) == SIGN_EXTEND
+	  || GET_CODE (x) == NEG))
     return CC_SWPmode;
 
   /* A compare of a mode narrower than SI mode against zero can be done
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 5d64228..db9eabc 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -1457,12 +1457,12 @@
 )
 
 (define_insn "*compare_neg<mode>"
-  [(set (reg:CC CC_REGNUM)
-	(compare:CC
-	 (match_operand:GPI 0 "register_operand" "r")
-	 (neg:GPI (match_operand:GPI 1 "register_operand" "r"))))]
+  [(set (reg:CC_SWP CC_REGNUM)
+	(compare:CC_SWP
+	 (neg:GPI (match_operand:GPI 0 "register_operand" "r"))
+	 (match_operand:GPI 1 "register_operand" "r")))]
   ""
-  "cmn\\t%<w>0, %<w>1"
+  "cmn\\t%<w>1, %<w>0"
   [(set_attr "v8type" "alus")
    (set_attr "mode" "<MODE>")]
 )
diff --git a/gcc/testsuite/gcc.target/aarch64/cmn-neg.c b/gcc/testsuite/gcc.target/aarch64/cmn-neg.c
new file mode 100644
index 0000000..05c8bbf
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/cmn-neg.c
@@ -0,0 +1,33 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps" } */
+
+extern void abort (void);
+
+void __attribute__ ((noinline))
+foo_s32 (int a, int b)
+{
+  if (a < -b)
+    abort ();
+}
+/* { dg-final { scan-assembler "cmn\tw\[0-9\]" } } */
+
+void __attribute__ ((noinline))
+foo_s64 (long long a, long long b)
+{
+  if (a < -b)
+    abort ();
+}
+/* { dg-final { scan-assembler "cmn\tx\[0-9\]" } } */
+
+
+int
+main (void)
+{
+  int a = 30;
+  int b = 42;
+  foo_s32 (a, b);
+  foo_s64 (a, b);
+  return 0;
+}
+
+/* { dg-final { cleanup-saved-temps } } */

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

* Re: [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9]
  2013-09-09 12:53   ` Kyrylo Tkachov
@ 2013-09-09 14:04     ` Richard Earnshaw
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Earnshaw @ 2013-09-09 14:04 UTC (permalink / raw)
  To: Kyrylo Tkachov; +Cc: gcc-patches, Marcus Shawcroft

On 09/09/13 13:50, Kyrylo Tkachov wrote:
> Hi Richard,
> 
>> On 29/07/13 14:58, Kyrylo Tkachov wrote:
>>> Hi all,
>>>
>>> Now that combine emits the canonical form for (eq (neg x) (y)) instead
>> of (eq
>>> (x) (neg y)), this patch fixes up the corresponding pattern in aarch64
>> to
>>> match that. This enables combine to properly generate the cmn
>> instruction
>>> where appropriate.
>>>
>>> Tested aarch64-none-elf on model.
>>>
>>> Ok for trunk?
>>>
>>
>> No, this is wrong for inequalities, since in reality it's the second
>> operand that's inverted.
>>
>> You'll need to use CC_SWP mode and then arrange for this to be correctly
>> picked by select_cc_mode.
>>
> 
> How's this?
> 
> aarch64_select_cc_mode is updated to return CC_SWPmode for these comparisons and
> the MD pattern is updated accordingly.
> 
> Tested aarch64-none-elf on a model.
> 
> Is this ok?
> 
> Thanks,
> Kyrill
> 
> 2013-09-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
> 	* config/aarch64/aarch64.c (aarch64_select_cc_mode): Return CC_SWP for
> 	comparison with negated operand.
> 	* config/aarch64/aarch64.md (compare_neg<mode>): Match canonical RTL form.
> 	
> 2013-09-09  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
> 	* gcc.target/aarch64/cmn-neg.c: New test.
> 
> 
> 

OK.

R.


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

end of thread, other threads:[~2013-09-09 13:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-29 14:06 [PATCH][AArch64] Fix FAIL: gcc.target/aarch64/cmn.c scan-assembler cmn\tw[0-9] Kyrylo Tkachov
2013-07-30 10:19 ` Richard Earnshaw
2013-09-09 12:53   ` Kyrylo Tkachov
2013-09-09 14:04     ` Richard Earnshaw

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