[PATCH] D154695: [Coroutines] Add an O(n) algorithm for computing the cross suspend point information.

[witstorm via Phabricator via llvm-commits]

witstorm95 added a comment.

In D154695#4492224 <https://reviews.llvm.org/D154695#4492224>, @ChuanqiXu wrote:

>> For code reuse, I don't really think split computeBlockData to 2 functions is good idea.
>
> My major concern is about the readability. The so called 1,2,3 iterations are confusing. It is magic numbers and reader may have a hard time to read them.
>
> Then out of curiosity, are the algorithm capable to handle the following case?
>
>   J -> I
>   I -> S
>   S -> I
>
> S is a suspend block. And `I.Kill[J]` should be false since the path from `J` to `I` that contains the suspend block must repeat node I. So it doesn't fit into the definition of `Kill`. I feel the algorithm doesn't get this right but I am super sure. It is better to give it a test case too.

Thanks. This is a good example. But this problem exists previous algorithm too(before this patch). Let me illustrate it for you,

Pick a visiting order:  J, I, S

Initial status:

  J Consumes: J         Kills:           Suspend: false   End: false     KillLoop: false    Changed: true
  I Consumes: I         Kills:           Suspend: false   End: false     KillLoop: false    Changed: true
  S Consumes: S         Kills: S         Suspend: true    End: false     KillLoop: false    Changed: true

Excuting computeBlockData</*Initialize=*/true>()

  Visit J, J has no predecessors.           Status of J Consumes: J         Kills:           Suspend: false   End: false     KillLoop: false    Changed: true
  Visit I, I has predecessors J, S. 
    So propagate J to I. After propagation, status of I Consumes: J, I         Kills:           Suspend: false   End: false     KillLoop: false    Changed: true
    So propagate S to I. After propagation, status of I Consumes: J, I, S         Kills: S         Suspend: false   End: false     KillLoop: false    Changed: true
                             After visit I, status of I Consumes: J, I, S         Kills: S         Suspend: false   End: false     KillLoop: false    Changed: true
  Visit S, S has predecessors I.
     So propagate I to S. After propagation status of S Consumes: J, I, S         Kills: J, I, S         Suspend: true    End: false     KillLoop: false    Changed: true

After execute computeBlockData</*Initialize=*/true>(), the status is:

  J Consumes: J               Kills:           Suspend: false   End: false     KillLoop: false    Changed: true
  I Consumes: J, I, S         Kills: S         Suspend: false   End: false     KillLoop: false    Changed: true
  S Consumes: J, I, S         Kills: J, I, S   Suspend: true    End: false     KillLoop: false    Changed: true

Executing while (computeBlockData());

  Visit J, J has no predecessors.            Status of J Consumes: J         Kills:           Suspend: false   End: false     KillLoop: false    Changed: false
  Visit I, I has predecessors J, S and predecessors have a change. 
     So propagate J to I. After propagation, status of I Consumes: J, I, S         Kills: S          Suspend: false   End: false     KillLoop: false    Changed: true
     So propagate S to I. After propagation, status of I Consumes: J, I, S         Kills: J, S         Suspend: false   End: false     KillLoop: true    Changed: true
                              After visit I, status of I Consumes: J, I, S         Kills: J, S         Suspend: false   End: false     KillLoop: true    Changed: true
   Visit S, S has predecessors I and predecessors have a change.
     So propagate I to S, after propagation status of S Consumes: J, I, S         Kills: J, I, S         Suspend: true    End: false     KillLoop: false    Changed: false

After execute computeBlockData</*Initialize=*/true>() firstly, the status is:

  J Consumes: J               Kills:              Suspend: false   End: false     KillLoop: false    Changed: false
  I Consumes: J, I, S         Kills: J, S         Suspend: false   End: false     KillLoop: true     Changed: true
  S Consumes: J, I, S         Kills: J, I, S      Suspend: true    End: false     KillLoop: false    Changed: false

> S is a suspend block. And `I.Kill[J]` should be false since the path from `J` to `I` that contains the suspend block must repeat node I. So it doesn't fit into the definition of `Kill`.

I think current result will be enough to prove it or there is something wrong with the proof process ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154695