From 58814445d4a43d9ab8a2adba5309d6affd378101 Mon Sep 17 00:00:00 2001 From: Sanjay Patel Date: Wed, 9 Jul 2014 16:34:54 +0000 Subject: [PATCH] Fix for PR20059 (instcombine reorders shufflevector after instruction that may trap) In PR20059 ( http://llvm.org/pr20059 ), instcombine eliminates shuffles that are necessary before performing an operation that can trap (srem). This patch calls isSafeToSpeculativelyExecute() and bails out of the optimization in SimplifyVectorOp() if needed. Differential Revision: http://reviews.llvm.org/D4424 llvm-svn: 212629 --- .../InstCombine/InstructionCombining.cpp | 6 ++++ llvm/test/Transforms/InstCombine/pr20059.ll | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 llvm/test/Transforms/InstCombine/pr20059.ll diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 6cba8e37509b..487699c41e95 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -42,6 +42,7 @@ #include "llvm/Analysis/ConstantFolding.h" #include "llvm/Analysis/InstructionSimplify.h" #include "llvm/Analysis/MemoryBuiltins.h" +#include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/CFG.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/GetElementPtrTypeIterator.h" @@ -1195,6 +1196,11 @@ static Value *CreateBinOpAsGiven(BinaryOperator &Inst, Value *LHS, Value *RHS, Value *InstCombiner::SimplifyVectorOp(BinaryOperator &Inst) { if (!Inst.getType()->isVectorTy()) return nullptr; + // It may not be safe to reorder shuffles and things like div, urem, etc. + // because we may trap when executing those ops on unknown vector elements. + // See PR20059. + if (!isSafeToSpeculativelyExecute(&Inst)) return nullptr; + unsigned VWidth = cast(Inst.getType())->getNumElements(); Value *LHS = Inst.getOperand(0), *RHS = Inst.getOperand(1); assert(cast(LHS->getType())->getNumElements() == VWidth); diff --git a/llvm/test/Transforms/InstCombine/pr20059.ll b/llvm/test/Transforms/InstCombine/pr20059.ll new file mode 100644 index 000000000000..8d7d69bf8765 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/pr20059.ll @@ -0,0 +1,32 @@ +; RUN: opt -S -instcombine < %s | FileCheck %s + +; In PR20059 ( http://llvm.org/pr20059 ), shufflevector operations are reordered/removed +; for an srem operation. This is not a valid optimization because it may cause a trap +; on div-by-zero. + +; CHECK-LABEL: @do_not_reorder +; CHECK: %splat1 = shufflevector <4 x i32> %p1, <4 x i32> undef, <4 x i32> zeroinitializer +; CHECK-NEXT: %splat2 = shufflevector <4 x i32> %p2, <4 x i32> undef, <4 x i32> zeroinitializer +; CHECK-NEXT: %retval = srem <4 x i32> %splat1, %splat2 +define <4 x i32> @do_not_reorder(<4 x i32> %p1, <4 x i32> %p2) { + %splat1 = shufflevector <4 x i32> %p1, <4 x i32> undef, <4 x i32> zeroinitializer + %splat2 = shufflevector <4 x i32> %p2, <4 x i32> undef, <4 x i32> zeroinitializer + %retval = srem <4 x i32> %splat1, %splat2 + ret <4 x i32> %retval +} +; RUN: opt -S -instcombine < %s | FileCheck %s + +; In PR20059 ( http://llvm.org/pr20059 ), shufflevector operations are reordered/removed +; for an srem operation. This is not a valid optimization because it may cause a trap +; on div-by-zero. + +; CHECK-LABEL: @do_not_reorder +; CHECK: %splat1 = shufflevector <4 x i32> %p1, <4 x i32> undef, <4 x i32> zeroinitializer +; CHECK-NEXT: %splat2 = shufflevector <4 x i32> %p2, <4 x i32> undef, <4 x i32> zeroinitializer +; CHECK-NEXT: %retval = srem <4 x i32> %splat1, %splat2 +define <4 x i32> @do_not_reorder(<4 x i32> %p1, <4 x i32> %p2) { + %splat1 = shufflevector <4 x i32> %p1, <4 x i32> undef, <4 x i32> zeroinitializer + %splat2 = shufflevector <4 x i32> %p2, <4 x i32> undef, <4 x i32> zeroinitializer + %retval = srem <4 x i32> %splat1, %splat2 + ret <4 x i32> %retval +}