[PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[Kyrill Tkachov]

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

Hi all,

An arm-none-eabi build with RTL checking and --with-cpu=cortex-a9 fails because
cortex_a9_sched_adjust_cost tries to access the SET_DEST of a PARALLEL.
The correct thing to do is to call single_set on dep, which will return a simple SET
that we can take the SET_DEST of or NULL if there's more than one SET.

This patch does that.
The arm-none-eabi build passes.
Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (cortex_a9_sched_adjust_cost): Use reg_set_p to
     check for dependencies.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm-ca9-checking.patch --]
[-- Type: text/x-patch; name=arm-ca9-checking.patch, Size: 918 bytes --]

commit bef3ba20485085d702b9a6c8683683db49b547f0
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Oct 27 18:09:29 2015 +0000

    [ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 08a852d..452b23d 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -11649,9 +11649,7 @@ cortex_a9_sched_adjust_cost (rtx_insn *insn, rtx link, rtx_insn *dep, int * cost
 		       case. However this gets modeled as an true
 		       dependency and hence all these checks.  */
 		    if (REG_P (SET_DEST (PATTERN (insn)))
-			&& REG_P (SET_DEST (PATTERN (dep)))
-			&& reg_overlap_mentioned_p (SET_DEST (PATTERN (insn)),
-						    SET_DEST (PATTERN (dep))))
+			&& reg_set_p (SET_DEST (PATTERN (insn)), dep))
 		      {
 			/* FMACS is a special case where the dependent
 			   instruction can be issued 3 cycles before

Re: [PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[Ramana Radhakrishnan]

On 29/10/15 16:02, Kyrill Tkachov wrote:
> Hi all,
> 
> An arm-none-eabi build with RTL checking and --with-cpu=cortex-a9 fails because
> cortex_a9_sched_adjust_cost tries to access the SET_DEST of a PARALLEL.
> The correct thing to do is to call single_set on dep, which will return a simple SET
> that we can take the SET_DEST of or NULL if there's more than one SET.
> 
> This patch does that.
> The arm-none-eabi build passes.
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/arm/arm.c (cortex_a9_sched_adjust_cost): Use reg_set_p to
>     check for dependencies.

Ok - but I think we also need a patch to improve the comment for reg_set_p, probably because it started life as internal function but now has wider visibility.


Thanks,
Ramana

Re: [PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[Kyrill Tkachov]

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


On 30/10/15 14:37, Ramana Radhakrishnan wrote:
>
> On 29/10/15 16:02, Kyrill Tkachov wrote:
>> Hi all,
>>
>> An arm-none-eabi build with RTL checking and --with-cpu=cortex-a9 fails because
>> cortex_a9_sched_adjust_cost tries to access the SET_DEST of a PARALLEL.
>> The correct thing to do is to call single_set on dep, which will return a simple SET
>> that we can take the SET_DEST of or NULL if there's more than one SET.
>>
>> This patch does that.
>> The arm-none-eabi build passes.
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      * config/arm/arm.c (cortex_a9_sched_adjust_cost): Use reg_set_p to
>>      check for dependencies.
> Ok - but I think we also need a patch to improve the comment for reg_set_p, probably because it started life as internal function but now has wider visibility.

Thanks,

Here's a patch for the reg_set_p comment.
Committing as obvious.

Kyrill

>
>
> Thanks,
> Ramana
>


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: reg_set_p_comment.patch --]
[-- Type: text/x-patch; name=reg_set_p_comment.patch, Size: 381 bytes --]

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 23b2c45..d7201ea 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -1207,7 +1207,8 @@ reg_set_between_p (const_rtx reg, const rtx_insn *from_insn,
   return 0;
 }
 
-/* Internals of reg_set_between_p.  */
+/* Return true if REG is set or clobbered inside INSN.  */
+
 int
 reg_set_p (const_rtx reg, const_rtx insn)
 {

Re: [PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[Kyrill Tkachov]

Ping.
https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03170.html

Thanks,
Kyrill
On 30/10/15 15:47, Kyrill Tkachov wrote:
>
> On 30/10/15 14:37, Ramana Radhakrishnan wrote:
>>
>> On 29/10/15 16:02, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> An arm-none-eabi build with RTL checking and --with-cpu=cortex-a9 fails because
>>> cortex_a9_sched_adjust_cost tries to access the SET_DEST of a PARALLEL.
>>> The correct thing to do is to call single_set on dep, which will return a simple SET
>>> that we can take the SET_DEST of or NULL if there's more than one SET.
>>>
>>> This patch does that.
>>> The arm-none-eabi build passes.
>>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>>
>>> Ok for trunk?
>>>
>>> Thanks,
>>> Kyrill
>>>
>>> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      * config/arm/arm.c (cortex_a9_sched_adjust_cost): Use reg_set_p to
>>>      check for dependencies.
>> Ok - but I think we also need a patch to improve the comment for reg_set_p, probably because it started life as internal function but now has wider visibility.
>
> Thanks,
>
> Here's a patch for the reg_set_p comment.
> Committing as obvious.
>
> Kyrill
>
>>
>>
>> Thanks,
>> Ramana
>>
>

Re: [PATCH][ARM] Fix checking RTL error in cortex_a9_sched_adjust_cost

[Kyrill Tkachov]

On 06/11/15 11:37, Kyrill Tkachov wrote:
> Ping.
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03170.html

My apologies, I meant to ping another patch.
Please disregard that.

Kyrill

>
> Thanks,
> Kyrill
> On 30/10/15 15:47, Kyrill Tkachov wrote:
>>
>> On 30/10/15 14:37, Ramana Radhakrishnan wrote:
>>>
>>> On 29/10/15 16:02, Kyrill Tkachov wrote:
>>>> Hi all,
>>>>
>>>> An arm-none-eabi build with RTL checking and --with-cpu=cortex-a9 fails because
>>>> cortex_a9_sched_adjust_cost tries to access the SET_DEST of a PARALLEL.
>>>> The correct thing to do is to call single_set on dep, which will return a simple SET
>>>> that we can take the SET_DEST of or NULL if there's more than one SET.
>>>>
>>>> This patch does that.
>>>> The arm-none-eabi build passes.
>>>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>>>
>>>> Ok for trunk?
>>>>
>>>> Thanks,
>>>> Kyrill
>>>>
>>>> 2015-10-29  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>>
>>>>      * config/arm/arm.c (cortex_a9_sched_adjust_cost): Use reg_set_p to
>>>>      check for dependencies.
>>> Ok - but I think we also need a patch to improve the comment for reg_set_p, probably because it started life as internal function but now has wider visibility.
>>
>> Thanks,
>>
>> Here's a patch for the reg_set_p comment.
>> Committing as obvious.
>>
>> Kyrill
>>
>>>
>>>
>>> Thanks,
>>> Ramana
>>>
>>
>