[PATCH] D87882: [AMDGPU] Fix merging m0 inits

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

In D87882#2282886 <https://reviews.llvm.org/D87882#2282886>, @rampitec wrote:

> In D87882#2282818 <https://reviews.llvm.org/D87882#2282818>, @piotr wrote:
>
>> In D87882#2282278 <https://reviews.llvm.org/D87882#2282278>, @rampitec wrote:
>>
>>> Doesn't loop block self dominate?
>>
>> You mean, why the condition "MDT.dominates(From, To)" returns false if From and To are in the same BB? Inside that function there is a bb dominance check if the instructions are in different blocks, but if the instructions are in the same block the code checks whether From comes before To.
>>
>> The bug I am trying to fix is simplified in the test m0-in-loop-0 where before my patch SI_INIT from line 333 was incorrectly removed. Although "m0 = COPY" comes after SI_INIT in that bb, this is a loop and in the second iteration DS_WRITE would see a clobbered m0, so SI_INIT has to be preserved.
>
> That sounds like a bug in the dominate() to me.



In D87882#2282886 <https://reviews.llvm.org/D87882#2282886>, @rampitec wrote:

> In D87882#2282818 <https://reviews.llvm.org/D87882#2282818>, @piotr wrote:
>
>> In D87882#2282278 <https://reviews.llvm.org/D87882#2282278>, @rampitec wrote:
>>
>>> Doesn't loop block self dominate?
>>
>> You mean, why the condition "MDT.dominates(From, To)" returns false if From and To are in the same BB? Inside that function there is a bb dominance check if the instructions are in different blocks, but if the instructions are in the same block the code checks whether From comes before To.
>>
>> The bug I am trying to fix is simplified in the test m0-in-loop-0 where before my patch SI_INIT from line 333 was incorrectly removed. Although "m0 = COPY" comes after SI_INIT in that bb, this is a loop and in the second iteration DS_WRITE would see a clobbered m0, so SI_INIT has to be preserved.
>
> That sounds like a bug in the dominate() to me.

MDT.dominates() gives correct answer. In the failing case "From" (m0 copy, line 335 in the test) is inside the same block as "To" (SI_INIT, line 333). "From" comes after "To", and "From" does not dominate "To" (so MDT.dominates() return false as expected), but "To" is reachable from "From" from previous iteration, so "To" cannot be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87882