From 3d4579829e85c108e729ade64d778e614b702e16 Mon Sep 17 00:00:00 2001 From: Neil Henning Date: Wed, 10 Oct 2018 09:27:45 +0000 Subject: [PATCH] Fix an ordering bug in the scalarizer. 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 --- llvm/lib/Transforms/Scalar/Scalarizer.cpp | 9 +++++++-- llvm/test/Transforms/Scalarizer/crash-bug.ll | 2 +- llvm/test/Transforms/Scalarizer/order-bug.ll | 23 +++++++++++++++++++++++ llvm/test/Transforms/Scalarizer/phi-bug.ll | 24 ++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 llvm/test/Transforms/Scalarizer/order-bug.ll create mode 100644 llvm/test/Transforms/Scalarizer/phi-bug.ll diff --git a/llvm/lib/Transforms/Scalar/Scalarizer.cpp b/llvm/lib/Transforms/Scalar/Scalarizer.cpp index 34ed126..2f873ab 100644 --- a/llvm/lib/Transforms/Scalar/Scalarizer.cpp +++ b/llvm/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 @@ 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 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; diff --git a/llvm/test/Transforms/Scalarizer/crash-bug.ll b/llvm/test/Transforms/Scalarizer/crash-bug.ll index 36430a9..95055d0 100644 --- a/llvm/test/Transforms/Scalarizer/crash-bug.ll +++ b/llvm/test/Transforms/Scalarizer/crash-bug.ll @@ -15,7 +15,7 @@ bb1: ; preds = %bb2, %0 %bb1_vec = phi <2 x i16> [ , %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: diff --git a/llvm/test/Transforms/Scalarizer/order-bug.ll b/llvm/test/Transforms/Scalarizer/order-bug.ll new file mode 100644 index 0000000..891c1eb --- /dev/null +++ b/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 +} diff --git a/llvm/test/Transforms/Scalarizer/phi-bug.ll b/llvm/test/Transforms/Scalarizer/phi-bug.ll new file mode 100644 index 0000000..03bffb9 --- /dev/null +++ b/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> + 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 +} -- 2.7.4