[CGP] Fix the remove of matched phis in complex addressing mode

[skatkov]

Summary

When we replace the Phi we created with matched ones it is possible that
there are two identical phi nodes in IR. And matcher is smart enough to find that
new created phi matches both of them. So we try to replace our phi node with
matched ones twice and what is bad we delete our phi node twice causing a crash.

As soon as we found that we have two identical Phi nodes it makes sense to do
a clean-up and replace one phi node by other one.
The patch implements it.

[skatkov]

ping.

[john.brawn]

lib/CodeGen/CodeGenPrepare.cpp

2952 ↗

(On Diff #135876)

MV.first has already been deleted by this point, and using the value of a deleted pointer is undefined behaviour (even if you're not dereferencing it). I think this can be solved by splitting this into two loops: one which just builds up DoneMatched (which would probably be clearer if you named it something like MatchedPHINodeMapping), and then a second which iterates over DoneMatched and does the actual replacing and erasing.

2955 ↗

(On Diff #135876)

identiacal

identical

[skatkov]

Please take a look.

[john.brawn]

LGTM.

llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp

while (PhiNodesToMatch.size()) {
         // If it does not match, collect all Phi nodes from matcher.
         // if we end up with no match, them all these Phi nodes will not match
         // later.
         for (auto M : Matched)
           WillNotMatch.insert(M.first);
         Matched.clear();
       }
       if (IsMatched) {
-        // Replace all matched values and erase them.
+        // If we matched phi node to different but identical phis then
+        // make a simplification here.
+        DenseMap<PHINode *, PHINode *> MatchedPHINodeMapping;
         for (auto MV : Matched) {
+          auto AlreadyMatched = MatchedPHINodeMapping.find(MV.first);
+          if (AlreadyMatched != MatchedPHINodeMapping.end()) {
+            MV.second->replaceAllUsesWith(AlreadyMatched->second);
+            ST.Put(MV.second, AlreadyMatched->second);
+            MV.second->eraseFromParent();
+          } else
+            MatchedPHINodeMapping.insert({ MV.first, MV.second });
+        }
+        // Replace all matched values and erase them.
+        for (auto MV : MatchedPHINodeMapping) {
           MV.first->replaceAllUsesWith(MV.second);
           PhiNodesToMatch.erase(MV.first);
           ST.Put(MV.first, MV.second);
           MV.first->eraseFromParent();
         }
         Matched.clear();
         continue;
       }

llvm/trunk/test/Transforms/CodeGenPrepare/X86/sink-addrmode-two-phi.ll

+; RUN: opt -S -codegenprepare -disable-complex-addr-modes=false  %s | FileCheck %s --check-prefix=CHECK
+target datalayout =
+"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test() {
+entry:
+  %0 = getelementptr inbounds i64, i64 * null, i64 undef
+  br label %start
+
+start:
+  %val1 = phi i64 * [ %0, %entry ], [ %val4, %exit ]
+  %val2 = phi i64 * [ null, %entry ], [ %val5, %exit ]
+  br i1 false, label %slowpath, label %exit
+
+slowpath:
+  %elem1 = getelementptr inbounds i64, i64 * undef, i64 undef
+  br label %exit
+
+exit:
+; CHECK: sunkaddr
+  %val3 = phi i64 * [ undef, %slowpath ], [ %val2, %start ]
+  %val4 = phi i64 * [ %elem1, %slowpath ], [ %val1, %start ]
+  %val5 = phi i64 * [ undef, %slowpath ], [ %val2, %start ]
+  %loadx = load i64, i64 * %val4, align 8
+  br label %start
+}