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 <li...@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 <li...@gcc.gnu.org>
>>
>> * ira.c (combine_and_move_insns): Skip multiple_sets def_insn.
>
>
