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

[Neil Henning via Phabricator via llvm-commits]

sheredom created this revision.
sheredom added reviewers: arsenm, mehdi_amini, uabelho, bkramer.
Herald added subscribers: llvm-commits, wdng.

I've added a new test case that causes the scalarizer to try and use dead-and-erased values - caused by the basic blocks not being in domination order within the function. To fix this, instead of iterating through the blocks in function order, I walk them in reverse post order.


Repository:
  rL LLVM

https://reviews.llvm.org/D52540

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


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.167090.patch
Type: text/x-patch
Size: 2607 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180926/2ea711b5/attachment-0001.bin>