[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