[PATCH] D91513: [DeadMachineInstrctionElim] Iteratively run DeadMachineInstructionElim pass until nothing dead

[Michael Liao via Phabricator via llvm-commits]

hliao added a comment.

In D91513#2398733 <https://reviews.llvm.org/D91513#2398733>, @rampitec wrote:

> In D91513#2398726 <https://reviews.llvm.org/D91513#2398726>, @hliao wrote:
>
>> In D91513#2398717 <https://reviews.llvm.org/D91513#2398717>, @rampitec wrote:
>>
>>> In D91513#2398697 <https://reviews.llvm.org/D91513#2398697>, @hliao wrote:
>>>
>>>> In D91513#2398528 <https://reviews.llvm.org/D91513#2398528>, @guopeilin wrote:
>>>>
>>>>> In D91513#2397313 <https://reviews.llvm.org/D91513#2397313>, @hliao wrote:
>>>>>
>>>>>> could you elaborate more on why we need to run that iteratively? since the original one runs bottom-up, supposedly it should find all.
>>>>>
>>>>> From the `iteratively-run-dead-mi-elim.mir` we can see that `bb.5` defines `%6`, and `%6` is used in `bb.2`. When we traverse the all basic blocks, that is, we runs bottome-up, we will meet `bb.5` first, for `%6`, we find that it is not dead cause  `%3` in `bb.2` use it. So `%6` surrive. Then we continue traverse other BBs, When we meet `bb.2`, we see that no one use `%5`, so we kill it. So as `%4`, `%3`. Right now, actually `%6` becomes dead cause we kill `%3` thus there is no longer any one uses `%6`. 
>>>>> However, cause we only traverse blocks once, we can't erase `%6` at the end. So if we iteratively visit all blocks until nothing change, then we can ensure that all dead mi is erased.
>>>>
>>>> Ah, I see. Even though we try to traverse basic blocks bottom-up, that's just the block placement order instead of block reachability. Could we replace that order with the post-order? So that, the use is always traversed before the defiine.
>>>
>>> That probably will not help if we have a loop?
>>
>> It still works unless that value has a cyclic dependency through phi-node.
>
> That's exactly what I had in mind, a phi node as the only way to get a cyclic dependency in SSA.
>
> I tend to say this is LGTM. Although I wish to see a test with a cyclic dependency.

That value with cyclic dependence cannot be removed unless that loop is deemed as dead. It's beyond the scope this DCE. Using the post order removes the need for iterative runs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91513