[PATCH] D91513: [DeadMachineInstrctionElim] Post order visit all blocks and Iteratively run DeadMachineInstructionElim pass until nothing dead

Wed Nov 18 08:15:56 PST 2020

hliao added a comment.

In D91513#2402005 <https://reviews.llvm.org/D91513#2402005>, @guopeilin wrote:

> In D91513#2401715 <https://reviews.llvm.org/D91513#2401715>, @hliao wrote:
>
>> In D91513#2401473 <https://reviews.llvm.org/D91513#2401473>, @guopeilin wrote:
>>
>>> In D91513#2401470 <https://reviews.llvm.org/D91513#2401470>, @guopeilin wrote:
>>>
>>>> In D91513#2400468 <https://reviews.llvm.org/D91513#2400468>, @hliao wrote:
>>>>
>>>>> Using post-order is quite straight-forward and only involves several lines of change. Please check the attachment. F14044934: 0001-DeadMachineInstructionElim-Use-post-order-for-basic-.patch <https://reviews.llvm.org/F14044934> That test passed with this traverse order change.
>>>>
>>>> That's a great help, I pass all my related cases with this patch, Thanks a lot.
>>>
>>> Now that we decide to use post order to visit all blocks of a function, I think we need to consider that what if CFG contains cycles?F14055467: DG with cycles.PNG <https://reviews.llvm.org/F14055467>
>>> From this picture, we can see that post order is not clearly defined cause there exits cycles, one of the possible orders is that `[ m, g, d, e, c, b, t, x]`
>>> So `m` comes before `g`, if we define something in `m` and use it in `g`. Then even though both def and use are useless, cause we visit `m` first, we will still get a dead definition after we post-order visit all blocks.
>>> So is it possible there still exist some cases theoretically that cannot be fixed by post-order visit? That is we may still need to iteratively run?
>>
>> You are right, that's possible. That case should be rare as that's a def in the back-edge with acyclic dep. Could you merge the post-order change together with the iterative runs? so that, in the regular case, we at most run twice. Please keep on eye on compile time.
>
> `Post order visit` and `iteratively run` are merged.

Do you add a test case similar to the CFG you showed?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91513/new/

https://reviews.llvm.org/D91513