[PATCH] D34397: [SCEV] Make MulOpsInlineThreshold lower to avoid excessive compilation time

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

In https://reviews.llvm.org/D34397#786272, @mkazantsev wrote:

> Sanjoy, actually merging operand Muls into patent Mul is O(N^2) by itself: you need a linear time to merge each of them (and if we sort them by time, it is N log(N)). So if you have a parent Mul of max ops amount, and which op is also a Mul node with max Ops amount, then oops - we will merge them for long.
>
> This is where we spend a lot of time in this particular test, but I don't exclude that there are other algorithms that will die on this step.


Maybe not the best explanation, let me be more specific.

  while (const SCEVMulExpr *Mul = dyn_cast<SCEVMulExpr>(Ops[Idx])) { 
    if (Ops.size() > MulOpsInlineThreshold)
      break;
    // If we have an mul, expand the mul operands onto the end of the
    // operands list.
    Ops.erase(Ops.begin()+Idx);                                    // Erase
    Ops.append(Mul->op_begin(), Mul->op_end());    // Append
    DeletedMul = true;
  }

If we have N ops on current level, each of these ops is a Mul with M ops (and N * M = MulOpsInlineThreshold), we:

1. Make N appends of M elements, with total cost N * M;
2. Make N erases, cost of each one is linear from number of ops on current level. On i-th iteration, we have N + M * (i - 1) ops on current level. The overall cost of these erases is O(sum(i = 1 ... N) (N + M * (i - 1))), which is O(N^2 + N*M).

The overall complexity is O(N^2 + N*M), which is extremely bad if (for example) N = 500, M = 2. Maybe we should consider rewriting this algorithm so that we don't make these erases.

And again, I don't see why similar problems shouldn't happen in other places.


https://reviews.llvm.org/D34397