[PATCH] D71578: [CodeMoverUtils] Improve IsControlFlowEquivalent.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added a comment.

> This is a quite deep introspection that I'd assume would be in the domain of some analysis, such as value numbering. It could ensure that equivalent conditional branches will take the same `llvm::Value`.

I agree that to make the ControlConditions class more powerful, we will need value numbering to check for equivalence between `llvm::Value`s. I am considering that as future improvement. And the code is written in a way that there should be only one function need to be modified once value numbering is available to be queried.

> However, if your use case is loop fusion, then it might not even be necessary. Loop fusion can be made strictly more powerful by sinking the loop condition into the loop. Another pass might then be able to optimize the fused interior if the conditions are equivalent. That is,
> 
>   if (c1)
>     for (int i = 0; i < n; ++i)
>       body1(i);
>   if (c2)
>     for (int i = 0; i < n; ++i)
>       body2(i);
>    
> 
> can be fused to
> 
>   for (int i = 0; i < n; ++i) {
>     if (c1)
>       body1(i);
>     if (c2)
>       body2(i);
>   }
> 
> 
> If `c1` is equivalent to `c2`, JumpThreading may change it to
> 
>   for (int i = 0; i < n; ++i) {
>     if (c1) {
>       body1(i);
>       body2(i);
>     }
>   }
> 
> 
> Hoisting `c1` out of the loop again, even if `c1` and `c2` stay separate would be a job for LoopUnswitching.
> 
> I realize (well, I did not in the call this morning) that fusing that loop fusion will probably not improve performance unless `c1` and `c2` usually evaluate to `true`, but at least for correctness (#pragma clang loop fuse), it is valid. Profitability can be checked separately.

To check for profitability, LoopFusion end up need to do something similar, e.g check if the two sets of control conditions it decided to sink in are equivalent, if not, then likely it is not profitable.
In addition, I imagine that it is not always possible to sink the conditions in the loop, e.g.

  if (c1)
    for (int i = 0; i < n; ++i)
      body1(i);
  if (c2)
    for (int i = A[x]; i < n; ++i)
      body2(i);

i in the second loop initialize with a LoadInst which is originally guarded by c2. `if(c2)` cannot be proven safe to move inside the loop, as the LoadInst could throw. Assuming A is not modified in the first loop, LoopFusion should still be able to fuse them, by moving the LoadInst to the preheader of the first loop, which by this patch can be proven control flow equivalent with the preheader of the second loop.

For LoopFusion, one use case of ControlConditions class is to check if it is safe to move intervening code around, while intervening code may not be safe to be moved out of a branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71578