[PATCH] D52540: Fix an ordering bug in the scalarizer.

[Neil Henning via Phabricator via llvm-commits]

sheredom updated this revision to Diff 167319.
sheredom added a comment.

Incorporate a related test case that my approach also fixes from https://bugs.llvm.org/show_bug.cgi?id=28911


https://reviews.llvm.org/D52540

Files:
  lib/Transforms/Scalar/Scalarizer.cpp
  test/Transforms/Scalarizer/crash-bug.ll
  test/Transforms/Scalarizer/order-bug.ll
  test/Transforms/Scalarizer/phi-bug.ll


Index: test/Transforms/Scalarizer/phi-bug.ll
===================================================================
--- /dev/null
+++ test/Transforms/Scalarizer/phi-bug.ll
@@ -0,0 +1,24 @@
+; RUN: opt %s -scalarizer -verify -S -o - | FileCheck %s
+
+define void @f3() local_unnamed_addr {
+bb1:
+  br label %bb2
+
+bb3:
+; CHECK-LABEL: bb3:
+; CHECK-NEXT: br label %bb4
+  %h.10.0.vec.insert = shufflevector <1 x i16> %h.10.1, <1 x i16> undef, <1 x i32> <i32 0>
+  br label %bb4
+
+bb2:
+; CHECK-LABEL: bb2:
+; CHECK: phi i16
+  %h.10.1 = phi <1 x i16> [ undef, %bb1 ]
+  br label %bb3
+
+bb4:
+; CHECK-LABEL: bb4:
+; CHECK: phi i16
+  %h.10.2 = phi <1 x i16> [ %h.10.0.vec.insert, %bb3 ]
+  ret void
+}
Index: test/Transforms/Scalarizer/order-bug.ll
===================================================================
--- /dev/null
+++ test/Transforms/Scalarizer/order-bug.ll
@@ -0,0 +1,23 @@
+; RUN: opt %s -scalarizer -S -o - | FileCheck %s
+
+; This input caused the scalarizer to replace & erase gathered results when 
+; future gathered results depended on them being alive
+
+define dllexport spir_func <4 x i32> @main(float %a) {
+entry:
+  %i = insertelement <4 x float> undef, float %a, i32 0
+  br label %z
+
+y:
+; CHECK: %f.upto0 = insertelement <4 x i32> undef, i32 %b.i0, i32 0
+; CHECK: %f.upto1 = insertelement <4 x i32> %f.upto0, i32 %b.i0, i32 1
+; CHECK: %f.upto2 = insertelement <4 x i32> %f.upto1, i32 %b.i0, i32 2
+; CHECK: %f = insertelement <4 x i32> %f.upto2, i32 %b.i0, i32 3
+  %f = shufflevector <4 x i32> %b, <4 x i32> undef, <4 x i32> zeroinitializer
+  ret <4 x i32> %f
+
+z:
+; CHECK: %b.i0 = bitcast float %a to i32
+  %b = bitcast <4 x float> %i to <4 x i32>
+  br label %y
+}
Index: test/Transforms/Scalarizer/crash-bug.ll
===================================================================
--- test/Transforms/Scalarizer/crash-bug.ll
+++ test/Transforms/Scalarizer/crash-bug.ll
@@ -15,7 +15,7 @@
   %bb1_vec = phi <2 x i16> [ <i16 100, i16 200>, %0 ], [ %bb2_vec, %bb2 ]
 ;CHECK: bb1:
 ;CHECK: %bb1_vec.i0 = phi i16 [ 100, %0 ], [ 0, %bb2 ]
-;CHECK: %bb1_vec.i1 = phi i16 [ 200, %0 ], [ %bb1_vec.i1, %bb2 ]
+;CHECK: %bb2_vec.i1 = phi i16 [ 200, %0 ], [ %bb2_vec.i1, %bb2 ]
   br i1 undef, label %bb3, label %bb2
 
 bb3:
Index: lib/Transforms/Scalar/Scalarizer.cpp
===================================================================
--- lib/Transforms/Scalar/Scalarizer.cpp
+++ lib/Transforms/Scalar/Scalarizer.cpp
@@ -14,6 +14,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Analysis/VectorUtils.h"
@@ -289,8 +290,12 @@
   if (skipFunction(F))
     return false;
   assert(Gathered.empty() && Scattered.empty());
-  for (BasicBlock &BB : F) {
-    for (BasicBlock::iterator II = BB.begin(), IE = BB.end(); II != IE;) {
+
+  // To ensure we replace gathered components correctly we need to do an ordered
+  // traversal of the basic blocks in the function.
+  ReversePostOrderTraversal<BasicBlock *> RPOT(&F.getEntryBlock());
+  for (BasicBlock *BB : RPOT) {
+    for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) {
       Instruction *I = &*II;
       bool Done = visit(I);
       ++II;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52540.167319.patch
Type: text/x-patch
Size: 3308 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180927/6ecd743b/attachment.bin>