[PATCH] D44571: [CGP] Avoid segmentation fault when doing PHI node simplifications

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 18 23:13:57 PDT 2018


skatkov added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2642
+  void ReplacePhi(PHINode *From, PHINode *To) {
+    if (!AllPhiNodes.count(From)) {
+      // The From pointer is probably dangling, so return here to make sure we
----------------
It can be an assert now (taking into account two loops in the place of invocation.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2986
+            // a "first" node)? Shouldn't we also make sure to remove MV.second
+            // from PhiNodesToMatch now when it is erased?
             MV.second->eraseFromParent();
----------------
Make sense however I guess that in general MV.second will never be in the PhiNodesToMatch.

The idea here that PhiNodesToMatch contains a Phi nodes created on previous steps. And they are matched against already existed Phi nodes before this algorithm started to work. So in general PhiNodesToMatch should not intersect with MV.second.

I guess assert will be ok in this case.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2992
         // Replace all matched values and erase them.
         for (auto MV : MatchedPHINodeMapping) {
+          ST.ReplacePhi(MV.first, MV.second);
----------------
you do not need {} now.


Repository:
  rL LLVM

https://reviews.llvm.org/D44571





More information about the llvm-commits mailing list