[PATCH] D99987: [NewGVN] Track simplification dependencies for phi-of-ops.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

I think it makes sense to add the users at the upper call level where the real instruction is known. Just one comment inline.



================
Comment at: llvm/lib/Transforms/Scalar/NewGVN.cpp:692
+      if (ExtraDep && ExtraDep != User && isa<Instruction>(ExtraDep))
+        AdditionalUsers[ExtraDep].insert(User);
+      ExtraDep = nullptr;
----------------
For consistency with the rest of the GVN APIs, I wound't add this functionality inside `ExprResult`.
I think an API akin to `addAdditionalUsers`, which queries the `ExprResult` for the dependency field and adds the user would maintain this consistency. It can even call `addAdditionalUsers` after the first two checks; and in the dependent patch, it would also call `addPredicateUsers`.
What do you think?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99987