[PATCH 6/8] Handle SCRATCH in decompose_address

[PATCH 6/8] Handle SCRATCH in decompose_address

[Maxim Kuvyrkov]

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

Hi,

This patch is a simple fix to allow decompose_address to handle SCRATCH'es during 2nd scheduler pass. This patch is a prerequisite for a scheduler improvement that relies on decompose_address to parse insns.

Bootstrapped and regtested on x86_64-linux-gnu and regtested on arm-linux-gnueabihf and aarch64-linux-gnu.

OK to apply?

Thank you,

[ChangeLog is part of the git patch]

--
Maxim Kuvyrkov
www.linaro.org



[-- Attachment #2: 0006-Handle-SCRATCH-in-decompose_address.patch --]
[-- Type: application/octet-stream, Size: 799 bytes --]

From 5d9f17d0a7532bdd3a9efbf5043de024514ce3b2 Mon Sep 17 00:00:00 2001
From: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Date: Fri, 4 Apr 2014 16:18:33 +1300
Subject: [PATCH 6/8] Handle SCRATCH in decompose_address

	* rtlanal.c (get_base_term): Handle SCRATCH.
---
 gcc/rtlanal.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 75362e4..e3353c2 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5774,7 +5774,8 @@ get_base_term (rtx *inner)
     inner = strip_address_mutations (&XEXP (*inner, 0));
   if (REG_P (*inner)
       || MEM_P (*inner)
-      || GET_CODE (*inner) == SUBREG)
+      || GET_CODE (*inner) == SUBREG
+      || GET_CODE (*inner) == SCRATCH)
     return inner;
   return 0;
 }
-- 
1.7.9.5

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Richard Sandiford]

Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> writes:
> This patch is a simple fix to allow decompose_address to handle
> SCRATCH'es during 2nd scheduler pass. This patch is a prerequisite for a
> scheduler improvement that relies on decompose_address to parse insns.
>
> Bootstrapped and regtested on x86_64-linux-gnu and regtested on
> arm-linux-gnueabihf and aarch64-linux-gnu.

Can't approve it, but FWIW, as the author of the original code it looks
good to me.  I agree (mem (scratch)) as an idiom for a mem with an
unknown address should be handled here.

Thanks,
Richard

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Jeff Law]

On 10/20/14 21:35, Maxim Kuvyrkov wrote:
> Hi,
>
> This patch is a simple fix to allow decompose_address to handle
> SCRATCH'es during 2nd scheduler pass. This patch is a prerequisite
> for a scheduler improvement that relies on decompose_address to parse
> insns.
>
> Bootstrapped and regtested on x86_64-linux-gnu and regtested on
> arm-linux-gnueabihf and aarch64-linux-gnu.
I'd like to see some further discussion here.

get_base_term is supposed to look at its argument as a base address. 
I'm curious under what circumstances you want to have a SCRATCH as a 
base address?

I didn't see anything in patch #8 which obviously dependended on this, 
but maybe it's in there, but more subtle than expected.

If you can justify why it's useful to handle scratch in here, then the 
patch will be fine.

jeff

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Maxim Kuvyrkov]

On Oct 23, 2014, at 9:02 AM, Jeff Law <law@redhat.com> wrote:

> On 10/20/14 21:35, Maxim Kuvyrkov wrote:
>> Hi,
>> 
>> This patch is a simple fix to allow decompose_address to handle
>> SCRATCH'es during 2nd scheduler pass. This patch is a prerequisite
>> for a scheduler improvement that relies on decompose_address to parse
>> insns.
>> 
>> Bootstrapped and regtested on x86_64-linux-gnu and regtested on
>> arm-linux-gnueabihf and aarch64-linux-gnu.
> I'd like to see some further discussion here.
> 
> get_base_term is supposed to look at its argument as a base address. I'm curious under what circumstances you want to have a SCRATCH as a base address?
> 
> I didn't see anything in patch #8 which obviously dependended on this, but maybe it's in there, but more subtle than expected.
> 
> If you can justify why it's useful to handle scratch in here, then the patch will be fine.

Without this patch decompose_address() ICEs during second scheduler pass on prologue instructions that usually have "(clobber (mem:BLK (scratch))".  The only reason for this patch is to prevent that fault and enable use of decompose_address during 2nd scheduler pass.

Does this answer your question, or are you looking for a more in-depth reason?

Thank you,

--
Maxim Kuvyrkov
www.linaro.org

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Jeff Law]

On 10/22/14 17:01, Maxim Kuvyrkov wrote:
> On Oct 23, 2014, at 9:02 AM, Jeff Law <law@redhat.com> wrote:
>
>> On 10/20/14 21:35, Maxim Kuvyrkov wrote:
>>> Hi,
>>>
>>> This patch is a simple fix to allow decompose_address to handle
>>> SCRATCH'es during 2nd scheduler pass. This patch is a
>>> prerequisite for a scheduler improvement that relies on
>>> decompose_address to parse insns.
>>>
>>> Bootstrapped and regtested on x86_64-linux-gnu and regtested on
>>> arm-linux-gnueabihf and aarch64-linux-gnu.
>> I'd like to see some further discussion here.
>>
>> get_base_term is supposed to look at its argument as a base
>> address. I'm curious under what circumstances you want to have a
>> SCRATCH as a base address?
>>
>> I didn't see anything in patch #8 which obviously dependended on
>> this, but maybe it's in there, but more subtle than expected.
>>
>> If you can justify why it's useful to handle scratch in here, then
>> the patch will be fine.
>
> Without this patch decompose_address() ICEs during second scheduler
> pass on prologue instructions that usually have "(clobber (mem:BLK
> (scratch))".  The only reason for this patch is to prevent that fault
> and enable use of decompose_address during 2nd scheduler pass.
>
> Does this answer your question, or are you looking for a more
> in-depth reason?
Yea, that's everything I needed to know.  Patch approved.

Thanks,
Jeff

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Maxim Kuvyrkov]

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

On Oct 23, 2014, at 4:18 AM, Jeff Law <law@redhat.com> wrote:

> On 10/22/14 17:01, Maxim Kuvyrkov wrote:
>> On Oct 23, 2014, at 9:02 AM, Jeff Law <law@redhat.com> wrote:
>> 
>>> On 10/20/14 21:35, Maxim Kuvyrkov wrote:
>>>> Hi,
>>>> 
>>>> This patch is a simple fix to allow decompose_address to handle
>>>> SCRATCH'es during 2nd scheduler pass. This patch is a
>>>> prerequisite for a scheduler improvement that relies on
>>>> decompose_address to parse insns.
>>>> 
>>>> Bootstrapped and regtested on x86_64-linux-gnu and regtested on
>>>> arm-linux-gnueabihf and aarch64-linux-gnu.
>>> I'd like to see some further discussion here.
>>> 
>>> get_base_term is supposed to look at its argument as a base
>>> address. I'm curious under what circumstances you want to have a
>>> SCRATCH as a base address?
>>> 
>>> I didn't see anything in patch #8 which obviously dependended on
>>> this, but maybe it's in there, but more subtle than expected.
>>> 
>>> If you can justify why it's useful to handle scratch in here, then
>>> the patch will be fine.
>> 
>> Without this patch decompose_address() ICEs during second scheduler
>> pass on prologue instructions that usually have "(clobber (mem:BLK
>> (scratch))".  The only reason for this patch is to prevent that fault
>> and enable use of decompose_address during 2nd scheduler pass.
>> 
>> Does this answer your question, or are you looking for a more
>> in-depth reason?
> Yea, that's everything I needed to know.  Patch approved.

Hi,

Turns out that the above patch applies without conflicts to two functions in rtlanal.c: get_base_term(), for which the patch is intended, and get_index_term(), for which the patch is not.

Due to git rebases and patch updates, I have accidentally pushed the patch twice and unintentionally changed get_index_term().  From what I can tell the change is benign, but, still, it is unnecessary.  The attached patch reverts the accidental commit.  It was bootstrapped arm-linux-gnueabihf.

OK for stage 1?  I'll regtest it before committing, just in case.

Thanks,

--
Maxim Kuvyrkov
www.linaro.org



[-- Attachment #2: 0001-Revert-accidental-commit-get_base_index-was-the-inte.patch --]
[-- Type: application/octet-stream, Size: 839 bytes --]

From 64cb581f9d2d1c1847b8dadd3acbf0d89080b7ac Mon Sep 17 00:00:00 2001
From: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Date: Tue, 27 Jan 2015 07:24:27 +0000
Subject: [PATCH] Revert accidental commit, get_base_index was the intended
 target

	* rtlanal.c (get_index_term): Revert accidental commit.
---
 gcc/rtlanal.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index 743aad6..915a66b 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -5704,8 +5704,7 @@ get_index_term (rtx *inner)
     inner = strip_address_mutations (&XEXP (*inner, 0));
   if (REG_P (*inner)
       || MEM_P (*inner)
-      || GET_CODE (*inner) == SUBREG
-      || GET_CODE (*inner) == SCRATCH)
+      || GET_CODE (*inner) == SUBREG)
     return inner;
   return 0;
 }
-- 
1.7.9.5

Re: [PATCH 6/8] Handle SCRATCH in decompose_address

[Jeff Law]

On 01/28/2015 12:03 AM, Maxim Kuvyrkov wrote:
> Hi,
>
> Turns out that the above patch applies without conflicts to two
> functions in rtlanal.c: get_base_term(), for which the patch is
> intended, and get_index_term(), for which the patch is not.
>
> Due to git rebases and patch updates, I have accidentally pushed the
> patch twice and unintentionally changed get_index_term().  From what
> I can tell the change is benign, but, still, it is unnecessary.  The
> attached patch reverts the accidental commit.  It was bootstrapped
> arm-linux-gnueabihf.
>
> OK for stage 1?  I'll regtest it before committing, just in case.
>
> Thanks,
>
> -- Maxim Kuvyrkov www.linaro.org
>
>
>
> 0001-Revert-accidental-commit-get_base_index-was-the-inte.patch
>
>
> From 64cb581f9d2d1c1847b8dadd3acbf0d89080b7ac Mon Sep 17 00:00:00
> 2001 From: Maxim Kuvyrkov<maxim.kuvyrkov@linaro.org> Date: Tue, 27
> Jan 2015 07:24:27 +0000 Subject: [PATCH] Revert accidental commit,
> get_base_index was the intended target
>
> * rtlanal.c (get_index_term): Revert accidental commit.

OK.
jeff