Skip to content

Commit

Permalink
Fix an ordering bug in the scalarizer.
Browse files Browse the repository at this point in the history
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.

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

llvm-svn: 344128
  • Loading branch information
Neil Henning committed Oct 10, 2018
1 parent 0c17cbf commit 3d45798
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 3 deletions.
9 changes: 7 additions & 2 deletions llvm/lib/Transforms/Scalar/Scalarizer.cpp
Expand Up @@ -14,6 +14,7 @@
//
//===----------------------------------------------------------------------===//

#include "llvm/ADT/PostOrderIterator.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Analysis/VectorUtils.h"
Expand Down Expand Up @@ -289,8 +290,12 @@ bool Scalarizer::runOnFunction(Function &F) {
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;
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/Scalarizer/crash-bug.ll
Expand Up @@ -15,7 +15,7 @@ bb1: ; preds = %bb2, %0
%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:
Expand Down
23 changes: 23 additions & 0 deletions llvm/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
}
24 changes: 24 additions & 0 deletions llvm/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
}

0 comments on commit 3d45798

Please sign in to comment.