PR target/48240

Message ID gkr1royxdqb.fsf@arm.com
State New
Headers show
Series
  • PR target/48240
Related show

Commit Message

Andrea Corallo April 8, 2020, 2:18 p.m.
Hi all,

I'd like to submit this for PR48240.

Bootstrapped on aarch64-unknown-linux-gnu.
Okay for trunk when finished with regression?

Andrea

gcc/ChangeLog

2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	PR target/48240
	* gcc/config/aarch64/falkor-tag-collision-avoidance.c
	(valid_src_p): Fix missing rtx type check.

gcc/testsuite/ChangeLog

2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	* gcc.target/aarch64/pr48240.c: New test.

Comments

Richard Sandiford April 8, 2020, 4:26 p.m. | #1
Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi all,

>

> I'd like to submit this for PR48240.

>

> Bootstrapped on aarch64-unknown-linux-gnu.

> Okay for trunk when finished with regression?


OK, but the PR number looks like a typo.  Also:

> Andrea

>

> gcc/ChangeLog

>

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	PR target/48240

> 	* gcc/config/aarch64/falkor-tag-collision-avoidance.c

> 	(valid_src_p): Fix missing rtx type check.


no gcc/ prefix here.

Thanks,
Richard

>

> gcc/testsuite/ChangeLog

>

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	* gcc.target/aarch64/pr48240.c: New test.

>

> From 246337341a8b58c61dc29d2198b244da737b3ef0 Mon Sep 17 00:00:00 2001

> From: Andrea Corallo <andrea.corallo@arm.com>

> Date: Wed, 8 Apr 2020 13:38:28 +0100

> Subject: [PATCH] pr48240

>

> ---

>  gcc/config/aarch64/falkor-tag-collision-avoidance.c | 9 ++++++---

>  gcc/testsuite/gcc.target/aarch64/pr48240.c          | 9 +++++++++

>  2 files changed, 15 insertions(+), 3 deletions(-)

>  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr48240.c

>

> diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> index 719df484ee61..4e07a43282f7 100644

> --- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> +++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> @@ -538,9 +538,12 @@ valid_src_p (rtx src, rtx_insn *insn, struct loop *loop, bool *pre_post,

>    if (!aarch64_classify_address (&addr, XEXP (x, 0), mode, true))

>      return false;

>  

> -  unsigned regno = REGNO (addr.base);

> -  if (global_regs[regno] || fixed_regs[regno])

> -    return false;

> +  if (REG_P (addr.base))

> +    {

> +      unsigned regno = REGNO (addr.base);

> +      if (global_regs[regno] || fixed_regs[regno])

> +	return false;

> +    }

>  

>    if (addr.type == ADDRESS_REG_WB)

>      {

> diff --git a/gcc/testsuite/gcc.target/aarch64/pr48240.c b/gcc/testsuite/gcc.target/aarch64/pr48240.c

> new file mode 100644

> index 000000000000..012af6afeca7

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/aarch64/pr48240.c

> @@ -0,0 +1,9 @@

> +/* { dg-do compile } */

> +/* { dg-options "-v -Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */

> +

> +extern void bar(const char *);

> +

> +void foo(void) {

> +  for (;;)

> +    bar("");

> +}
Kyrylo Tkachov April 8, 2020, 4:33 p.m. | #2
Hi Andrea

> -----Original Message-----

> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Andrea

> Corallo

> Sent: 08 April 2020 15:19

> To: gcc-patches@gcc.gnu.org

> Cc: nd <nd@arm.com>

> Subject: [PATCH] PR target/48240

> 

> Hi all,

> 

> I'd like to submit this for PR48240.

> 

> Bootstrapped on aarch64-unknown-linux-gnu.

> Okay for trunk when finished with regression?

> 

> Andrea

> 

> gcc/ChangeLog

> 

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

> 

> 	PR target/48240

> 	* gcc/config/aarch64/falkor-tag-collision-avoidance.c

> 	(valid_src_p): Fix missing rtx type check.

> 

> gcc/testsuite/ChangeLog

> 

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

> 

> 	* gcc.target/aarch64/pr48240.c: New test.


From 246337341a8b58c61dc29d2198b244da737b3ef0 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>

Date: Wed, 8 Apr 2020 13:38:28 +0100
Subject: [PATCH] pr48240

---
 gcc/config/aarch64/falkor-tag-collision-avoidance.c | 9 ++++++---
 gcc/testsuite/gcc.target/aarch64/pr48240.c          | 9 +++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr48240.c

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index 719df484ee61..4e07a43282f7 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,9 +538,12 @@ valid_src_p (rtx src, rtx_insn *insn, struct loop *loop, bool *pre_post,
   if (!aarch64_classify_address (&addr, XEXP (x, 0), mode, true))
     return false;
 
-  unsigned regno = REGNO (addr.base);
-  if (global_regs[regno] || fixed_regs[regno])
-    return false;
+  if (REG_P (addr.base))
+    {
+      unsigned regno = REGNO (addr.base);
+      if (global_regs[regno] || fixed_regs[regno])
+	return false;
+    }

I think we want to just return false here if !REG_P (addr.base) rather than fall through?

 
   if (addr.type == ADDRESS_REG_WB)
     {
diff --git a/gcc/testsuite/gcc.target/aarch64/pr48240.c b/gcc/testsuite/gcc.target/aarch64/pr48240.c
new file mode 100644
index 000000000000..012af6afeca7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr48240.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-v -Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */

We shouldn't need the "-v" here...

Ok with those changes.
Thanks,
Kyrill

+
+extern void bar(const char *);
+
+void foo(void) {
+  for (;;)
+    bar("");
+}
-- 
2.17.1
Andrea Corallo April 9, 2020, 7:26 a.m. | #3
Richard Sandiford <richard.sandiford@arm.com> writes:

> Andrea Corallo <andrea.corallo@arm.com> writes:

>> Hi all,

>>

>> I'd like to submit this for PR48240.

>>

>> Bootstrapped on aarch64-unknown-linux-gnu.

>> Okay for trunk when finished with regression?

>

> OK, but the PR number looks like a typo.  Also:


Ooops, no idea how did I managed to generated this number.

>> Andrea

>>

>> gcc/ChangeLog

>>

>> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>

>> 	PR target/48240

>> 	* gcc/config/aarch64/falkor-tag-collision-avoidance.c

>> 	(valid_src_p): Fix missing rtx type check.

>

> no gcc/ prefix here.

>

> Thanks,

> Richard
Andrea Corallo April 9, 2020, 7:55 a.m. | #4
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

> diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> index 719df484ee61..4e07a43282f7 100644

> --- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> +++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> @@ -538,9 +538,12 @@ valid_src_p (rtx src, rtx_insn *insn, struct loop *loop, bool *pre_post,

>    if (!aarch64_classify_address (&addr, XEXP (x, 0), mode, true))

>      return false;

>  

> -  unsigned regno = REGNO (addr.base);

> -  if (global_regs[regno] || fixed_regs[regno])

> -    return false;

> +  if (REG_P (addr.base))

> +    {

> +      unsigned regno = REGNO (addr.base);

> +      if (global_regs[regno] || fixed_regs[regno])

> +	return false;

> +    }

>

> I think we want to just return false here if !REG_P (addr.base) rather than fall through?


I think functionally would be equivalent cause after we guard on
addr.type, but probably nicer.

> +++ b/gcc/testsuite/gcc.target/aarch64/pr48240.c

> @@ -0,0 +1,9 @@

> +/* { dg-do compile } */

> +/* { dg-options "-v -Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */

>

> We shouldn't need the "-v" here...


Ack

> Ok with those changes.

> Thanks,

> Kyrill


Thanks for reviewing updating the patch!

  Andrea

Patch

From 246337341a8b58c61dc29d2198b244da737b3ef0 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Wed, 8 Apr 2020 13:38:28 +0100
Subject: [PATCH] pr48240

---
 gcc/config/aarch64/falkor-tag-collision-avoidance.c | 9 ++++++---
 gcc/testsuite/gcc.target/aarch64/pr48240.c          | 9 +++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr48240.c

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index 719df484ee61..4e07a43282f7 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,9 +538,12 @@  valid_src_p (rtx src, rtx_insn *insn, struct loop *loop, bool *pre_post,
   if (!aarch64_classify_address (&addr, XEXP (x, 0), mode, true))
     return false;
 
-  unsigned regno = REGNO (addr.base);
-  if (global_regs[regno] || fixed_regs[regno])
-    return false;
+  if (REG_P (addr.base))
+    {
+      unsigned regno = REGNO (addr.base);
+      if (global_regs[regno] || fixed_regs[regno])
+	return false;
+    }
 
   if (addr.type == ADDRESS_REG_WB)
     {
diff --git a/gcc/testsuite/gcc.target/aarch64/pr48240.c b/gcc/testsuite/gcc.target/aarch64/pr48240.c
new file mode 100644
index 000000000000..012af6afeca7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr48240.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-v -Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */
+
+extern void bar(const char *);
+
+void foo(void) {
+  for (;;)
+    bar("");
+}
-- 
2.17.1