From 3839fd16a1a90de302e847109522379472108c30 Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 4 Nov 2014 23:49:08 +0000 Subject: [PATCH] Analysis: Make isSafeToSpeculativelyExecute fire less for divides Divides and remainder operations do not behave like other operations when they are given poison: they turn into undefined behavior. It's really hard to know if the operands going into a div are or are not poison. Because of this, we should only choose to speculate if there are constant operands which we can easily reason about. This fixes PR21412. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@221318 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/ValueTracking.cpp | 38 +++++++++++++++++++------------ test/Transforms/LICM/speculate.ll | 10 ++++---- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index 3e8cb933a7a..0d93285a0d6 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -2549,23 +2549,31 @@ bool llvm::isSafeToSpeculativelyExecute(const Value *V, default: return true; case Instruction::UDiv: - case Instruction::URem: - // x / y is undefined if y == 0, but calculations like x / 3 are safe. - return isKnownNonZero(Inst->getOperand(1), TD); + case Instruction::URem: { + // x / y is undefined if y == 0. + const APInt *V; + if (match(Inst->getOperand(1), m_APInt(V))) + return *V != 0; + return false; + } case Instruction::SDiv: case Instruction::SRem: { - Value *Op = Inst->getOperand(1); - // x / y is undefined if y == 0 - if (!isKnownNonZero(Op, TD)) - return false; - // x / y might be undefined if y == -1 - unsigned BitWidth = getBitWidth(Op->getType(), TD); - if (BitWidth == 0) - return false; - APInt KnownZero(BitWidth, 0); - APInt KnownOne(BitWidth, 0); - computeKnownBits(Op, KnownZero, KnownOne, TD); - return !!KnownZero; + // x / y is undefined if y == 0 or x == INT_MIN and y == -1 + const APInt *X, *Y; + if (match(Inst->getOperand(1), m_APInt(Y))) { + if (*Y != 0) { + if (*Y == -1) { + // The numerator can't be MinSignedValue if the denominator is -1. + if (match(Inst->getOperand(0), m_APInt(X))) + return !Y->isMinSignedValue(); + // The numerator *might* be MinSignedValue. + return false; + } + // The denominator is not 0 or -1, it's safe to proceed. + return true; + } + } + return false; } case Instruction::Load: { const LoadInst *LI = cast(Inst); diff --git a/test/Transforms/LICM/speculate.ll b/test/Transforms/LICM/speculate.ll index 4244f157d9f..69266693c47 100644 --- a/test/Transforms/LICM/speculate.ll +++ b/test/Transforms/LICM/speculate.ll @@ -3,12 +3,11 @@ ; UDiv is safe to speculate if the denominator is known non-zero. ; CHECK-LABEL: @safe_udiv( -; CHECK: %div = udiv i64 %x, %or +; CHECK: %div = udiv i64 %x, 2 ; CHECK-NEXT: br label %for.body define void @safe_udiv(i64 %x, i64 %m, i64 %n, i32* %p, i64* %q) nounwind { entry: - %or = or i64 %m, 1 br label %for.body for.body: ; preds = %entry, %for.inc @@ -19,7 +18,7 @@ for.body: ; preds = %entry, %for.inc br i1 %tobool, label %for.inc, label %if.then if.then: ; preds = %for.body - %div = udiv i64 %x, %or + %div = udiv i64 %x, 2 %arrayidx1 = getelementptr inbounds i64* %q, i64 %i.02 store i64 %div, i64* %arrayidx1, align 8 br label %for.inc @@ -69,13 +68,12 @@ for.end: ; preds = %for.inc, %entry ; known to have at least one zero bit. ; CHECK-LABEL: @safe_sdiv( -; CHECK: %div = sdiv i64 %x, %or +; CHECK: %div = sdiv i64 %x, 2 ; CHECK-NEXT: br label %for.body define void @safe_sdiv(i64 %x, i64 %m, i64 %n, i32* %p, i64* %q) nounwind { entry: %and = and i64 %m, -3 - %or = or i64 %and, 1 br label %for.body for.body: ; preds = %entry, %for.inc @@ -86,7 +84,7 @@ for.body: ; preds = %entry, %for.inc br i1 %tobool, label %for.inc, label %if.then if.then: ; preds = %for.body - %div = sdiv i64 %x, %or + %div = sdiv i64 %x, 2 %arrayidx1 = getelementptr inbounds i64* %q, i64 %i.02 store i64 %div, i64* %arrayidx1, align 8 br label %for.inc