[PATCH] D87063: [BitcodeReader] Fix O(N^2) in placeholder replacement algorithm.
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 24 18:41:20 PDT 2020
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
More information about the llvm-commits
mailing list