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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 22 14:30:13 PDT 2021


fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/NewGVN.cpp:692
+      if (ExtraDep && ExtraDep != User && isa<Instruction>(ExtraDep))
+        AdditionalUsers[ExtraDep].insert(User);
+      ExtraDep = nullptr;
----------------
asbirlea wrote:
> 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?
> 
Thanks, I think that makes a lot of sense! I updated the patch to use a new `addAdditionalUsers` variant which takes a `ExprResult` argument. I'll update the other patches tomorrow, if that looks good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99987



More information about the llvm-commits mailing list