[PATCH] D40166: [CGP] Fix the crash caused by enable of complex addr mode

[Serguei Katkov via Phabricator via llvm-commits]

skatkov created this revision.

We must collect all AddModes even if they are the same.
This is due to Original value is different but we need all original
values collected as they are used as anchors in common phi finding.


https://reviews.llvm.org/D40166

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/Transforms/CodeGenPrepare/X86/sink-addrmode-base.ll


Index: test/Transforms/CodeGenPrepare/X86/sink-addrmode-base.ll
===================================================================
--- test/Transforms/CodeGenPrepare/X86/sink-addrmode-base.ll
+++ test/Transforms/CodeGenPrepare/X86/sink-addrmode-base.ll
@@ -473,3 +473,40 @@
   %v = load i64 , i64* %p1, align 8
   ret i64 %v
 }
+
+; The same two addr modes by different paths
+define i32 @test18(i1 %cond1, i1 %cond2, i64* %b1, i64* %b2) {
+; CHECK-LABEL: @test18
+entry:
+  %g1 = getelementptr inbounds i64, i64* %b2, i64 5
+  %bc1 = bitcast i64* %g1 to i32*
+  br i1 %cond1, label %if.then1, label %if.then2
+
+if.then1:
+  %g2 = getelementptr inbounds i64, i64* %b1, i64 5
+  %bc2 = bitcast i64* %g2 to i32*
+  br label %fallthrough
+
+if.then2:
+  %bc1_1 = bitcast i64* %g1 to i32*
+  br i1 %cond2, label %fallthrough, label %if.then3
+
+if.then3:
+  %bc1_2 = bitcast i64* %g1 to i32*
+  br label %fallthrough
+
+fallthrough:
+; CHECK-YES: sunk_phi
+; CHECK-NO-LABEL: fallthrough:
+; CHECK-NO: phi
+; CHECK-NO-NEXT: load
+  %c = phi i32* [%bc2, %if.then1], [%bc1_1, %if.then2], [%bc1_2, %if.then3]
+  %v1 = load i32, i32* %c, align 4
+  %g1_1 = getelementptr inbounds i64, i64* %b2, i64 5
+  %bc1_1_1 = bitcast i64* %g1_1 to i32*
+  %v2 = load i32, i32* %bc1_1_1, align 4
+  %v = add i32 %v1, %v2
+  ret i32 %v
+}
+
+declare void @foo()
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -2800,15 +2800,11 @@
     else if (DifferentField != ThisDifferentField)
       DifferentField = ExtAddrMode::MultipleFields;
 
-    // If this AddrMode is the same as all the others then everything is fine
-    // (which should only happen when there is actually only one AddrMode).
-    if (DifferentField == ExtAddrMode::NoField) {
-      assert(AddrModes.size() == 1);
-      return true;
-    }
-
     // If NewAddrMode differs in only one dimension then we can handle it by
-    // inserting a phi/select later on.
+    // inserting a phi/select later on. Even if NewAddMode is the same
+    // we still need to collect it due to original value is different.
+    // And later we will need all original values as anchors during
+    // finding the common Phi node.
     if (DifferentField != ExtAddrMode::MultipleFields) {
       AddrModes.emplace_back(NewAddrMode);
       return true;
@@ -2829,7 +2825,7 @@
       return false;
 
     // A single AddrMode can trivially be combined.
-    if (AddrModes.size() == 1)
+    if (AddrModes.size() == 1 || DifferentField == ExtAddrMode::NoField)
       return true;
 
     // If the AddrModes we collected are all just equal to the value they are


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40166.123292.patch
Type: text/x-patch
Size: 2719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171117/c7d0ad96/attachment.bin>