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

[Pavel Samolysov via Phabricator via llvm-commits]

psamolysov added a comment.

Colleagues, @nikic Excuse me if I spend your time. I've had a look at the loop of searching the load instructions (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp#L563-L597) again and I cannot figure out an example where a cycle between an instruction and its user is. There is a loop over a worklist to find the dead instructions in the same pass: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp#L392-L416 and no `Visited` set is used in this loop at all (the code is very similar but the code on lines 563-597, I proposed a patch for, adds some condition checks in the case where the GEP instruction is visited). I tried to replace the type of the `Worklist` variable in my patch with `SmallVector` instead of `SmallSetVector` and all the `insert_range` operations with `append_range` and no test in the `llvm\test\Transforms\ArgumentPromotion` folder fell. This can be a proof that the `Visited` set is not needed or we just have no test to get an infinite loop over the worklist in the case where a cycle between an instruction and its user is possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123669