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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

@psamolysov The main thing we're concerned about when we use these "Visited" sets is unreachable code, which can include self-referencing instructions. You can have something like `%ptr = load ptr, ptr %ptr` and then loop infinitely.

Now, I think you are correct that this can't happen in this case, because we are only looking through instructions that have a single non-constant operand. However, if this code is ever changed to also handle other instruction kinds, one would have to be careful to reintroduce the visited set. For example, the "full restrict" patches add an additional operand to load instructions, which could be used to form a cycle in unreachable code.

So, I think it's safe to drop this Visited set in this case, but I would usually consider it good practice to have one, in the interest of defensiveness. Though I don't care particularly strongly about this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123669