[PATCH] D87063: [BitcodeReader] Fix O(N^2) in placeholder replacement algorithm.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

OK, having a go at reviewing this (though likely you're about in the best position to assess this, Eli)...

So, the original code:

  For each constant placeholder
    While there are still uses of the constant placeholder
      if the use isn't a constant, just change it to use the real value
      if the use is a constant
        walk all the operands of the user & update any that refer to placeholders
        make a new constant with the new list of operands
    replace the remaining non-constant users 

You mention:

> The key here is to make sure we never RAUW a constant that's used by other constants.

Though my naive reading of the existing code looks to me like it checks all its users and fixes them up before RAUW the remaining... ah, but that's not done recursively, so if a user itself has users that are constants - then there's a RAUW of a constant that's used by other constants. OK

Right, so the goal is to start with constants that aren't used by any other constants - I guess that means starting with something in `ResolveConstants` that has a value 0/not present in `NumRewrittenOperands`?

So we start by walking all the constant users of placeholder constants - and then also (line 174) the constant users of any of those (so I guess they're not all using placeholders - some are already resolved?)

Then nah, this is where I start to not be able to keep it all in my head/clearly. Could you perhaps explain in more detail to summarize the algorithm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87063