* [PATCH, IRA] Fix PR91052 by skipping multiple_sets insn in combine_and_move_insns
@ 2020-02-11 8:01 Kewen.Lin
2020-02-11 16:24 ` Vladimir Makarov
0 siblings, 1 reply; 3+ messages in thread
From: Kewen.Lin @ 2020-02-11 8:01 UTC (permalink / raw)
To: GCC Patches; +Cc: vmakarov, bergner, zadeck, Segher Boessenkool, Bill Schmidt
[-- Attachment #1: Type: text/plain, Size: 2189 bytes --]
Hi,
As PR91052's comments show, commit r272731 exposed one issue in function
combine_and_move_insns. Function combine_and_move_insns perform the
below unexpected transformation.
** Before: **
67: NOTE_INSN_BASIC_BLOCK 8
...
59: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;} ==> move object
REG_UNUSED r121:SI
77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
60: r122:SI=r127:SI
REG_DEAD r127:SI
61: [r122:SI]=r184:SF
REG_DEAD r184:SF
79: [++r122:SI]=r191:SF
REG_DEAD r191:SF
REG_INC r122:SI
64: r187:SF=[r137:SI+low(`*.LC0')]
99: r198:SF=[++r121:SI] =====> with sp-0x18c+4;
REG_INC r121:SI
104: r201:SF=[r137:SI+low(`*.LC0')]
65: [r126:SI]=r187:SF
REG_DEAD r187:SF
105: [r126:SI]=r201:SF
REG_DEAD r201:SF
101: [++r122:SI]=r198:SF
REG_DEAD r198:SF
REG_INC r122:SI
114: L114:
113: NOTE_INSN_BASIC_BLOCK 9
** After: **
67: NOTE_INSN_BASIC_BLOCK 8
...
77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
REG_UNUSED r121:SI
60: r122:SI=r127:SI
REG_DEAD r127:SI
219: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;} ==> moved here but update origin r121.
61: [r122:SI]=r184:SF
REG_DEAD r184:SF
79: [++r122:SI]=r191:SF
REG_DEAD r191:SF
REG_INC r122:SI
64: r187:SF=[r137:SI+low(`*.LC0')]
REG_EQUIV [r137:SI+low(`*.LC0')]
99: r198:SF=[++r121:SI] =====> with sp-0x18c; inconsistent from above.
REG_INC r121:SI
104: r201:SF=[r137:SI+low(`*.LC0')]
REG_EQUIV [r137:SI+low(`*.LC0')]
65: [r126:SI]=r187:SF
REG_DEAD r187:SF
105: [r126:SI]=r201:SF
REG_DEAD r201:SF
101: [++r122:SI]=r198:SF
REG_DEAD r198:SF
REG_INC r122:SI
114: L114:
113: NOTE_INSN_BASIC_BLOCK 9
The insn 59 is special with multiple_sets, its movement alters the live
interval of r121 from insn 77 to insn 99 and update r121 with unexpected
value.
Bootstrapped/regtested on powerpc64le-linux-gnu (LE) and
ppc64-redhat-linux (BE).
Is it ok for trunk?
BR,
Kewen
-------
gcc/ChangeLog
2020-02-11 Kewen Lin <linkw@gcc.gnu.org>
* ira.c (combine_and_move_insns): Skip multiple_sets def_insn.
[-- Attachment #2: PR91052.diff --]
[-- Type: text/plain, Size: 566 bytes --]
diff --git a/gcc/ira.c b/gcc/ira.c
index c8b5f86..a655ae1 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -3784,6 +3784,11 @@ combine_and_move_insns (void)
if (can_throw_internal (def_insn))
continue;
+ /* Instructions with multiple sets can only be moved if DF analysis is
+ performed for all of the registers set. See PR91052. */
+ if (multiple_sets (def_insn))
+ continue;
+
basic_block use_bb = BLOCK_FOR_INSN (use_insn);
basic_block def_bb = BLOCK_FOR_INSN (def_insn);
if (bb_loop_depth (use_bb) > bb_loop_depth (def_bb))
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, IRA] Fix PR91052 by skipping multiple_sets insn in combine_and_move_insns
2020-02-11 8:01 [PATCH, IRA] Fix PR91052 by skipping multiple_sets insn in combine_and_move_insns Kewen.Lin
@ 2020-02-11 16:24 ` Vladimir Makarov
2020-02-12 5:34 ` Kewen.Lin
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Makarov @ 2020-02-11 16:24 UTC (permalink / raw)
To: Kewen.Lin, GCC Patches; +Cc: bergner, zadeck, Segher Boessenkool, Bill Schmidt
On 2/11/20 3:01 AM, Kewen.Lin wrote:
> Hi,
>
> As PR91052's comments show, commit r272731 exposed one issue in function
> combine_and_move_insns. Function combine_and_move_insns perform the
> below unexpected transformation.
>
> ** Before: **
>
> 67: NOTE_INSN_BASIC_BLOCK 8
> ...
> 59: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;} ==> move object
> REG_UNUSED r121:SI
> 77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
> 60: r122:SI=r127:SI
> REG_DEAD r127:SI
> 61: [r122:SI]=r184:SF
> REG_DEAD r184:SF
> 79: [++r122:SI]=r191:SF
> REG_DEAD r191:SF
> REG_INC r122:SI
> 64: r187:SF=[r137:SI+low(`*.LC0')]
> 99: r198:SF=[++r121:SI] =====> with sp-0x18c+4;
> REG_INC r121:SI
> 104: r201:SF=[r137:SI+low(`*.LC0')]
> 65: [r126:SI]=r187:SF
> REG_DEAD r187:SF
> 105: [r126:SI]=r201:SF
> REG_DEAD r201:SF
> 101: [++r122:SI]=r198:SF
> REG_DEAD r198:SF
> REG_INC r122:SI
> 114: L114:
> 113: NOTE_INSN_BASIC_BLOCK 9
>
> ** After: **
>
> 67: NOTE_INSN_BASIC_BLOCK 8
> ...
> 77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
> REG_UNUSED r121:SI
> 60: r122:SI=r127:SI
> REG_DEAD r127:SI
> 219: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;} ==> moved here but update origin r121.
> 61: [r122:SI]=r184:SF
> REG_DEAD r184:SF
> 79: [++r122:SI]=r191:SF
> REG_DEAD r191:SF
> REG_INC r122:SI
> 64: r187:SF=[r137:SI+low(`*.LC0')]
> REG_EQUIV [r137:SI+low(`*.LC0')]
> 99: r198:SF=[++r121:SI] =====> with sp-0x18c; inconsistent from above.
> REG_INC r121:SI
> 104: r201:SF=[r137:SI+low(`*.LC0')]
> REG_EQUIV [r137:SI+low(`*.LC0')]
> 65: [r126:SI]=r187:SF
> REG_DEAD r187:SF
> 105: [r126:SI]=r201:SF
> REG_DEAD r201:SF
> 101: [++r122:SI]=r198:SF
> REG_DEAD r198:SF
> REG_INC r122:SI
> 114: L114:
> 113: NOTE_INSN_BASIC_BLOCK 9
>
> The insn 59 is special with multiple_sets, its movement alters the live
> interval of r121 from insn 77 to insn 99 and update r121 with unexpected
> value.
>
> Bootstrapped/regtested on powerpc64le-linux-gnu (LE) and
> ppc64-redhat-linux (BE).
>
> Is it ok for trunk?
Yes. Thank you for working on the PR, Kewen.
I don't think that any expensive additional analysis is worth to use it
for solving the PR. So I believe your patch is an adequate solution.
> -------
>
> gcc/ChangeLog
>
> 2020-02-11 Kewen Lin <linkw@gcc.gnu.org>
>
> * ira.c (combine_and_move_insns): Skip multiple_sets def_insn.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH, IRA] Fix PR91052 by skipping multiple_sets insn in combine_and_move_insns
2020-02-11 16:24 ` Vladimir Makarov
@ 2020-02-12 5:34 ` Kewen.Lin
0 siblings, 0 replies; 3+ messages in thread
From: Kewen.Lin @ 2020-02-12 5:34 UTC (permalink / raw)
To: Vladimir Makarov
Cc: GCC Patches, bergner, zadeck, Segher Boessenkool, Bill Schmidt
on 2020/2/12 ä¸å12:24, Vladimir Makarov wrote:
> On 2/11/20 3:01 AM, Kewen.Lin wrote:
>> Hi,
>>
>> As PR91052's comments show, commit r272731 exposed one issue in function
>> combine_and_move_insns. Function combine_and_move_insns perform the
>> below unexpected transformation.
>>
>> ** Before: **
>>
>> Â Â Â 67: NOTE_INSN_BASIC_BLOCK 8
>> ...
>> Â Â Â 59: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;}Â ==> move object
>> Â Â Â Â Â Â REG_UNUSED r121:SI
>> Â Â Â 77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
>> Â Â Â 60: r122:SI=r127:SI
>> Â Â Â Â Â Â REG_DEAD r127:SI
>> Â Â Â 61: [r122:SI]=r184:SF
>> Â Â Â Â Â Â REG_DEAD r184:SF
>> Â Â Â 79: [++r122:SI]=r191:SF
>> Â Â Â Â Â Â REG_DEAD r191:SF
>> Â Â Â Â Â Â REG_INC r122:SI
>> Â Â Â 64: r187:SF=[r137:SI+low(`*.LC0')]
>> Â Â Â 99: r198:SF=[++r121:SI]Â Â Â Â Â Â Â Â Â Â Â Â =====> with sp-0x18c+4;
>> Â Â Â Â Â Â REG_INC r121:SI
>> Â Â 104: r201:SF=[r137:SI+low(`*.LC0')]
>> Â Â Â 65: [r126:SI]=r187:SF
>> Â Â Â Â Â Â REG_DEAD r187:SF
>> Â Â 105: [r126:SI]=r201:SF
>> Â Â Â Â Â Â REG_DEAD r201:SF
>> Â Â 101: [++r122:SI]=r198:SF
>> Â Â Â Â Â Â REG_DEAD r198:SF
>> Â Â Â Â Â Â REG_INC r122:SI
>> Â Â 114: L114:
>> Â Â 113: NOTE_INSN_BASIC_BLOCK 9
>>
>> ** After: **
>>
>> Â Â Â 67: NOTE_INSN_BASIC_BLOCK 8
>> ...
>> Â Â Â 77: {r191:SF=[sfp:SI-0x18c];r121:SI=sfp:SI-0x18c;}
>> Â Â Â Â Â Â REG_UNUSED r121:SI
>> Â Â Â 60: r122:SI=r127:SI
>> Â Â Â Â Â Â REG_DEAD r127:SI
>> Â Â 219: {r184:SF=[sfp:SI-0x190];r121:SI=sfp:SI-0x190;}Â Â ==> moved here but update origin r121.
>> Â Â Â 61: [r122:SI]=r184:SF
>> Â Â Â Â Â Â REG_DEAD r184:SF
>> Â Â Â 79: [++r122:SI]=r191:SF
>> Â Â Â Â Â Â REG_DEAD r191:SF
>> Â Â Â Â Â Â REG_INC r122:SI
>> Â Â Â 64: r187:SF=[r137:SI+low(`*.LC0')]
>> Â Â Â Â Â Â REG_EQUIV [r137:SI+low(`*.LC0')]
>> Â Â Â 99: r198:SF=[++r121:SI]Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â =====> with sp-0x18c; inconsistent from above.
>> Â Â Â Â Â Â REG_INC r121:SI
>> Â Â 104: r201:SF=[r137:SI+low(`*.LC0')]
>> Â Â Â Â Â Â REG_EQUIV [r137:SI+low(`*.LC0')]
>> Â Â Â 65: [r126:SI]=r187:SF
>> Â Â Â Â Â Â REG_DEAD r187:SF
>> Â Â 105: [r126:SI]=r201:SF
>> Â Â Â Â Â Â REG_DEAD r201:SF
>> Â Â 101: [++r122:SI]=r198:SF
>> Â Â Â Â Â Â REG_DEAD r198:SF
>> Â Â Â Â Â Â REG_INC r122:SI
>> Â Â 114: L114:
>> Â Â 113: NOTE_INSN_BASIC_BLOCK 9
>>
>> The insn 59 is special with multiple_sets, its movement alters the live
>> interval of r121 from insn 77 to insn 99 and update r121 with unexpected
>> value.
>>
>> Bootstrapped/regtested on powerpc64le-linux-gnu (LE) and
>> ppc64-redhat-linux (BE).
>>
>> Is it ok for trunk?
>
> Yes. Thank you for working on the PR, Kewen.
>
> I don't think that any expensive additional analysis is worth to use it for solving the PR. So I believe your patch is an adequate solution.
>
Thanks Vladimir! Committed in r10-6591-g4d2248bec5d22061ab252724bd59d45c8a47e009
with the below updated ChangeLog (sorry for missing one PR line).
2020-02-12 Kewen Lin <linkw@gcc.gnu.org>
PR target/91052
* ira.c (combine_and_move_insns): Skip multiple_sets def_insn.
And yes, I doubt the gain with more expensive analysis to legalize
the movement, even with that we need to update notes like REG_UNUSED
(inconsistent notes is direct cause of the ICE), it also seems not trivial.
BR,
Kewen
>
>> -------
>>
>> gcc/ChangeLog
>>
>> 2020-02-11 Kewen Lin <linkw@gcc.gnu.org>
>>
>> Â Â Â Â * ira.c (combine_and_move_insns): Skip multiple_sets def_insn.
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-12 5:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 8:01 [PATCH, IRA] Fix PR91052 by skipping multiple_sets insn in combine_and_move_insns Kewen.Lin
2020-02-11 16:24 ` Vladimir Makarov
2020-02-12 5:34 ` Kewen.Lin
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).