From: Mikael Holmen <mikael.holmen@ericsson.com>
Date: Fri, 15 Mar 2019 13:51:05 +0000 (+0000)
Subject: [CodeGenPrepare]  avoid crashing from replacing a phi twice
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bc7dbf662a6574984e79e33331bbb1c80b4b5c7b;p=llvm

[CodeGenPrepare]  avoid crashing from replacing a phi twice

Summary:
This is a fix to bug 41052:
https://bugs.llvm.org/show_bug.cgi?id=41052

While trying to optimize a memory instruction in a dead basic block, we end up registering the same phi for replacement twice. This patch avoids registering more than the first replacement candidate for a phi.

Patch by: JesperAntonsson

Reviewers: skatkov, aprantl

Reviewed By: aprantl

Subscribers: jdoerfert, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D59358

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@356260 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp
index e8498eb375e..44f1709b6e9 100644
--- a/lib/CodeGen/CodeGenPrepare.cpp
+++ b/lib/CodeGen/CodeGenPrepare.cpp
@@ -3282,6 +3282,8 @@ private:
                     PhiNodeSet &PhiNodesToMatch) {
     SmallVector<PHIPair, 8> WorkList;
     Matcher.insert({ PHI, Candidate });
+    SmallSet<PHINode *, 8> MatchedPHIs;
+    MatchedPHIs.insert(PHI);
     WorkList.push_back({ PHI, Candidate });
     SmallSet<PHIPair, 8> Visited;
     while (!WorkList.empty()) {
@@ -3314,8 +3316,10 @@ private:
         if (Matcher.count({ FirstPhi, SecondPhi }))
           continue;
         // So the values are different and does not match. So we need them to
-        // match.
-        Matcher.insert({ FirstPhi, SecondPhi });
+        // match. (But we register no more than one match per PHI node, so that
+        // we won't later try to replace them twice.)
+        if (!MatchedPHIs.insert(FirstPhi).second)
+          Matcher.insert({ FirstPhi, SecondPhi });
         // But me must check it.
         WorkList.push_back({ FirstPhi, SecondPhi });
       }
diff --git a/test/CodeGen/X86/codegen-prepare-replacephi.mir b/test/CodeGen/X86/codegen-prepare-replacephi.mir
new file mode 100644
index 00000000000..476f0473098
--- /dev/null
+++ b/test/CodeGen/X86/codegen-prepare-replacephi.mir
@@ -0,0 +1,45 @@
+# RUN: llc -run-pass=codegenprepare -o - %s | FileCheck %s
+
+# This testcase without the accompanying fix triggers the assert
+# "Replacement PHI node is already replaced."
+
+--- |
+  define void @f1() {
+  entry:
+    %arrayidx = getelementptr inbounds [2 x i16], [2 x i16]* undef, i16 0, i16 2
+    %0 = bitcast i16* %arrayidx to i32*
+    %1 = bitcast [2 x i16]* undef to i32*
+    br label %for.cond
+
+  for.cond:
+    %2 = phi i32* [ %0, %entry ], [ %7, %cleanup ]
+    %3 = phi i32* [ %0, %entry ], [ %9, %cleanup ]
+    br label %for.body
+
+  for.body:
+    %4 = phi i32* [ %3, %for.cond ], [ %9, %cleanup ]
+    %5 = phi i32* [ %2, %for.cond ], [ %9, %cleanup ]
+    %6 = phi i32* [ %2, %for.cond ], [ %9, %cleanup ]
+    br i1 false, label %for.cond2, label %if.then
+
+  if.then:
+    store i32 undef, i32* %4, align 1
+    unreachable
+
+  for.cond2:
+    %7 = phi i32* [ %6, %for.body ], [ %7, %if.then5 ], [ %1, %for.cond2 ]
+    %8 = phi i32* [ %5, %for.body ], [ %8, %if.then5 ], [ %1, %for.cond2 ]
+    %9 = phi i32* [ %4, %for.body ], [ %8, %if.then5 ], [ %1, %for.cond2 ]
+    br i1 undef, label %for.cond2, label %if.then5
+
+  if.then5:
+    br i1 undef, label %cleanup, label %for.cond2
+
+  cleanup:
+    br i1 true, label %for.cond, label %for.body
+  }
+
+...
+
+# Sanity check to verify that something got through.
+# CHECK-LABEL: entry: