[PATCH] PR target/48240

[PATCH] PR target/48240

[Andrea Corallo]

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

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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-pr48240.patch --]
[-- Type: text/x-diff, Size: 1633 bytes --]

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

Re: [PATCH] PR target/48240

[Richard Sandiford]

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("");
> +}

Re: [PATCH] PR target/48240

[Andrea Corallo]

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

RE: [PATCH] PR target/48240

[Kyrylo Tkachov]

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

Re: [PATCH] PR target/48240

[Andrea Corallo]

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

[committed] PR target/94530 (was [PATCH] PR target/48240)

[Andrea Corallo]

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

Hi all,

Second version of the patch for PR94530 (pr num fixed) addressing 
comments.

Bootstrapped on aarch64-unknown-linux-gnu.

Committed.

Andrea

gcc/ChangeLog

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

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

gcc/testsuite/ChangeLog

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-pr94530.patch --]
[-- Type: text/x-diff, Size: 1459 bytes --]

From 6136519229b43ad7a9d0d304fd42280a6757493c 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] pr94530

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

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index 719df484ee61..f850153fae02 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,6 +538,9 @@ 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;
 
+  if (!REG_P (addr.base))
+    return false;
+
   unsigned regno = REGNO (addr.base);
   if (global_regs[regno] || fixed_regs[regno])
     return false;
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94530.c b/gcc/testsuite/gcc.target/aarch64/pr94530.c
new file mode 100644
index 000000000000..1f98201c50a8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr94530.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */
+
+extern void bar(const char *);
+
+void foo(void) {
+  for (;;)
+    bar("");
+}
-- 
2.17.1

Re: [committed] PR target/94530 (was [PATCH] PR target/48240)

[Christophe Lyon]

Hi,

On Thu, 9 Apr 2020 at 10:57, Andrea Corallo <andrea.corallo@arm.com> wrote:
>
> Hi all,
>
> Second version of the patch for PR94530 (pr num fixed) addressing
> comments.
>
> Bootstrapped on aarch64-unknown-linux-gnu.
>
> Committed.
>

The new test causes an ICE:

FAIL: gcc.target/aarch64/pr94530.c (internal compiler error)
FAIL: gcc.target/aarch64/pr94530.c (test for excess errors)
Excess errors:
during RTL pass: tag_collision_avoidance
/gcc/testsuite/gcc.target/aarch64/pr94530.c:9:1: internal compiler
error: Segmentation fault
0xd58bcf crash_signal
        /gcc/toplev.c:328
0x11d47f3 valid_src_p
        /gcc/config/aarch64/falkor-tag-collision-avoidance.c:541
0x11d47f3 get_load_info
        /gcc/config/aarch64/falkor-tag-collision-avoidance.c:644
0x11d58c7 record_loads
        /gcc/config/aarch64/falkor-tag-collision-avoidance.c:774
0x11d58c7 execute_tag_collision_avoidance()
        /gcc/config/aarch64/falkor-tag-collision-avoidance.c:827
0x11d6588 pass_tag_collision_avoidance::execute(function*)
        /gcc/config/aarch64/falkor-tag-collision-avoidance.c:874

Can you check?

Thanks,

Christophe

> Andrea
>
> gcc/ChangeLog
>
> 2020-04-09  Andrea Corallo  <andrea.corallo@arm.com>
>
>         PR target/pr94530
>         * config/aarch64/falkor-tag-collision-avoidance.c
>         (valid_src_p): Fix missing rtx type check.
>
> gcc/testsuite/ChangeLog
>
> 2020-04-09  Andrea Corallo  <andrea.corallo@arm.com>
>
>         * gcc.target/aarch64/pr94530.c: New test.
>

[PATCH]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

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

Hi all,

Testing the backport of pr94530 fix (af19e4d0e23e) to GCC 9 I realized
this is not entirely correct.

aarch64_classify_address sets the base field of struct
aarch64_address_info for all aarch64_address_type with the exception
of ADDRESS_SYMBOLIC.  In this case we would access an uninitialized
value.  The attached patch fixes the issue.

Bootstraped on aarch64 and regressioned.  Okay for trunk?

I've also tested af19e4d0e23e + the current patch rebased on on top of
the gcc-9 branch.  Okay to backport?

Andrea

PS I'm wondering if would be good also to always set the addr field in
aarch64_classify_address for safeness.

gcc/ChangeLog

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

	* config/aarch64/falkor-tag-collision-avoidance.c
	(valid_src_p): Check for aarch64_address_info type before
	accessing base field.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-pr94530-2.patch --]
[-- Type: text/x-diff, Size: 909 bytes --]

From 4af5bb32cd88bb4e65591207839fb0e276b2eb23 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Thu, 9 Apr 2020 15:34:50 +0100
Subject: [PATCH] pr94530 2

---
 gcc/config/aarch64/falkor-tag-collision-avoidance.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index f850153fae02..fb56ff66bfab 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,7 +538,7 @@ 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;
 
-  if (!REG_P (addr.base))
+  if (addr.type == ADDRESS_SYMBOLIC || !REG_P (addr.base))
     return false;
 
   unsigned regno = REGNO (addr.base);
-- 
2.17.1

RE: [PATCH]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Kyrylo Tkachov]

Hi Andrea,

> -----Original Message-----
> From: Andrea Corallo <Andrea.Corallo@arm.com>
> Sent: 14 April 2020 11:01
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; nd <nd@arm.com>;
> Christophe Lyon <christophe.lyon@linaro.org>
> Subject: [PATCH]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for
> use of uninitialized value
> 
> Hi all,
> 
> Testing the backport of pr94530 fix (af19e4d0e23e) to GCC 9 I realized
> this is not entirely correct.
> 
> aarch64_classify_address sets the base field of struct
> aarch64_address_info for all aarch64_address_type with the exception
> of ADDRESS_SYMBOLIC.  In this case we would access an uninitialized
> value.  The attached patch fixes the issue.
> 
> Bootstraped on aarch64 and regressioned.  Okay for trunk?
> 
> I've also tested af19e4d0e23e + the current patch rebased on on top of
> the gcc-9 branch.  Okay to backport?
> 
> Andrea
> 
> PS I'm wondering if would be good also to always set the addr field in
> aarch64_classify_address for safeness.
> 
> gcc/ChangeLog
> 
> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	* config/aarch64/falkor-tag-collision-avoidance.c
> 	(valid_src_p): Check for aarch64_address_info type before
> 	accessing base field.

From 4af5bb32cd88bb4e65591207839fb0e276b2eb23 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Thu, 9 Apr 2020 15:34:50 +0100
Subject: [PATCH] pr94530 2

---
 gcc/config/aarch64/falkor-tag-collision-avoidance.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index f850153fae02..fb56ff66bfab 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,7 +538,7 @@ 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;
 
-  if (!REG_P (addr.base))
+  if (addr.type == ADDRESS_SYMBOLIC || !REG_P (addr.base))
     return false;
 
Hmm, given that the code below only handles the  ADDRESS_REG_* forms, how about we get defensive here and check that addr.type is not one of ADDRESS_REG_* instead?
That way, if aarch64_classify_address is extended in the future with new addr.type values that leave addr.base in the wrong forms, this code will not need to be updated.
Thanks,
Kyrill


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

Re: [PATCH]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

> -  if (!REG_P (addr.base))
> +  if (addr.type == ADDRESS_SYMBOLIC || !REG_P (addr.base))
>      return false;
>
> Hmm, given that the code below only handles the  ADDRESS_REG_* forms, how about we get defensive here and check that addr.type is not one of ADDRESS_REG_* instead?
> That way, if aarch64_classify_address is extended in the future with new addr.type values that leave addr.base in the wrong forms, this code will not need to be updated.

Yeah make sense.

Thanks

  Andrea

Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

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

Hi all,

Second version of this addressing comments.

Bootstraped on aarch64 and regressioned.  Okay for trunk?

Andrea

gcc/ChangeLog

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

	* config/aarch64/falkor-tag-collision-avoidance.c
	(valid_src_p): Check for aarch64_address_info type before
	accessing base field.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-pr94530-3.patch --]
[-- Type: text/x-diff, Size: 738 bytes --]

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index f850153fae02..a96a3320e8fc 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -538,7 +538,11 @@ 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;
 
-  if (!REG_P (addr.base))
+  if (addr.type != ADDRESS_REG_IMM
+      && addr.type != ADDRESS_REG_WB
+      && addr.type != ADDRESS_REG_REG
+      && addr.type != ADDRESS_REG_UXTW
+      && addr.type != ADDRESS_REG_SXTW)
     return false;
 
   unsigned regno = REGNO (addr.base);

RE: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Kyrylo Tkachov]

Hi Andrea,

> -----Original Message-----
> From: Andrea Corallo <Andrea.Corallo@arm.com>
> Sent: 15 April 2020 09:47
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Cc: nd <nd@arm.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix
> valid_src_p for use of uninitialized value
> 
> Hi all,
> 
> Second version of this addressing comments.
> 
> Bootstraped on aarch64 and regressioned.  Okay for trunk?

Ok.
Thanks,
Kyrill

> 
> Andrea
> 
> gcc/ChangeLog
> 
> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	* config/aarch64/falkor-tag-collision-avoidance.c
> 	(valid_src_p): Check for aarch64_address_info type before
> 	accessing base field.

Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

> Hi Andrea,
>
>> -----Original Message-----
>> From: Andrea Corallo <Andrea.Corallo@arm.com>
>> Sent: 15 April 2020 09:47
>> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
>> Cc: nd <nd@arm.com>; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix
>> valid_src_p for use of uninitialized value
>>
>> Hi all,
>>
>> Second version of this addressing comments.
>>
>> Bootstraped on aarch64 and regressioned.  Okay for trunk?
>
> Ok.
> Thanks,
> Kyrill

Thanks pushed as 8a4436d89bfa.

Preparing the backport for gcc-9.

  Andrea

Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

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

Hi all,

I'd like to back-port this to the gcc-9 branch.
This patch is directly based on:

https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543627.html
https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543901.html

Bootstrapped and reg tested.  Ok for release/gcc-9?

Bests
  Andrea

gcc/ChangeLog

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

	* config/aarch64/falkor-tag-collision-avoidance.c
	(valid_src_p): Check for aarch64_address_info type before
	accessing base field.

gcc/testsuite/ChangeLog

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-pr94530-gcc-9.patch --]
[-- Type: text/x-diff, Size: 1638 bytes --]

From 1e70131c2c099c1071baba3f40d610f41ff4e9ea Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Tue, 14 Apr 2020 19:51:54 +0100
Subject: [PATCH] pr94530 gcc-9

---
 gcc/config/aarch64/falkor-tag-collision-avoidance.c | 7 +++++++
 gcc/testsuite/gcc.target/aarch64/pr94530.c          | 9 +++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/pr94530.c

diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index 779dee81f7f4..698d1595d0a6 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -537,6 +537,13 @@ 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;
 
+  if (addr.type != ADDRESS_REG_IMM
+      && addr.type != ADDRESS_REG_WB
+      && addr.type != ADDRESS_REG_REG
+      && addr.type != ADDRESS_REG_UXTW
+      && addr.type != ADDRESS_REG_SXTW)
+    return false;
+
   unsigned regno = REGNO (addr.base);
   if (global_regs[regno] || fixed_regs[regno])
     return false;
diff --git a/gcc/testsuite/gcc.target/aarch64/pr94530.c b/gcc/testsuite/gcc.target/aarch64/pr94530.c
new file mode 100644
index 000000000000..1f98201c50a8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr94530.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -mcpu=falkor -mpc-relative-literal-loads -mcmodel=large" } */
+
+extern void bar(const char *);
+
+void foo(void) {
+  for (;;)
+    bar("");
+}
-- 
2.17.1

RE: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Kyrylo Tkachov]

> -----Original Message-----
> From: Andrea Corallo <Andrea.Corallo@arm.com>
> Sent: 16 April 2020 09:01
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Cc: nd <nd@arm.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix
> valid_src_p for use of uninitialized value
> 
> Hi all,
> 
> I'd like to back-port this to the gcc-9 branch.
> This patch is directly based on:
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543627.html
> https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543901.html
> 
> Bootstrapped and reg tested.  Ok for release/gcc-9?

Ok.
Thanks,
Kyrill

> 
> Bests
>   Andrea
> 
> gcc/ChangeLog
> 
> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	* config/aarch64/falkor-tag-collision-avoidance.c
> 	(valid_src_p): Check for aarch64_address_info type before
> 	accessing base field.
> 
> gcc/testsuite/ChangeLog
> 
> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>
> 
> 	* gcc.target/aarch64/pr94530.c: New test.

Re: [PATCH V2]aarch64: falkor-tag-collision-avoidance.c fix valid_src_p for use of uninitialized value

[Andrea Corallo]

Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

>> Bootstrapped and reg tested.  Ok for release/gcc-9?
>
> Ok.
> Thanks,
> Kyrill

Pushed as 5e022e3b3f7b.

Thanks
  Andrea