[PATCH] D123669: [ArgPromotion] Use SmallSetVector to traverse values

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D123669#3448600 <https://reviews.llvm.org/D123669#3448600>, @psamolysov wrote:

> @nikic thank you very much for your comment. As I see in the removed code, in the `AppendUsers` lambda, every user of the value `V` is firstly inserted into the `Visited` set and if and only if the insertion takes place, the object is also appended to the Worklist vector. The `SmallSetVector` class implements exactly the same logic: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/SetVector.h#L141 (however, one difference is there, `SmallSetVector` uses `SmallDenceSet`, not `SmallPtrSet`). The problem I see, during the traversing, we remove elements from the end of the vector using it as a stack (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp#L573) and `SmallSetVector` will remove the elements from the set too, not only from the vector. So (in the new implementation), if a user was inserted into the worklist and then is taken for processing, the user is also removed from the set of visited elements and therefore can be added to the worklist again and again if there is a loop between the users. Is this your concern?

Yes, exactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123669