[llvm] r221318 - Analysis: Make isSafeToSpeculativelyExecute fire less for divides
Nick Lewycky
nicholas at mxc.ca
Sat Nov 8 13:24:01 PST 2014
David Majnemer wrote:
> It's a correctness issue.
>
> In general one cannot know whether or not a divide instruction is safe
> to execute because isKnownNonZero returns true for poison.
>
> It is possible for the right hand side of the divide to always be poison
> in a valid program so long as the divide cannot ever be executed.
Actually wait, how does that happen in practice? You need to have
already had an integer overflow or something to trigger poison, right?
>
> For instructions which trap, we need a stronger predicate than
> 'isKnownNonZero': we need a hypothetical 'isKnownNeverToBeUndef'.
>
> I doubt a correct implementation of isKnownNeverToBeUndef will ever fire
> because it must be incredibly conservative: function arguments might
> silently cary poison in them making them unsafe.
>
> On Wed, Nov 5, 2014 at 5:05 PM, Philip Reames <listmail at philipreames.com
> <mailto:listmail at philipreames.com>> wrote:
>
> David,
>
> Is this a correctness issue or a heuristic? The description doesn't
> say.
>
> I find this change potentially concerning since it will heavily
> influence LICM behaviour.
>
> Philip
>
>
> On 11/04/2014 03:49 PM, David Majnemer wrote:
>
> Author: majnemer
> Date: Tue Nov 4 17:49:08 2014
> New Revision: 221318
>
> URL: http://llvm.org/viewvc/llvm- project?rev=221318&view=rev
> <http://llvm.org/viewvc/llvm-project?rev=221318&view=rev>
> Log:
> 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.
>
> Modified:
> llvm/trunk/lib/Analysis/ ValueTracking.cpp
> llvm/trunk/test/Transforms/ LICM/speculate.ll
>
> Modified: llvm/trunk/lib/Analysis/ ValueTracking.cpp
> URL: http://llvm.org/viewvc/llvm- project/llvm/trunk/lib/
> Analysis/ValueTracking.cpp? rev=221318&r1=221317&r2=
> 221318&view=diff
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=221318&r1=221317&r2=221318&view=diff>
> ============================== ==============================
> ==================
> --- llvm/trunk/lib/Analysis/ ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ ValueTracking.cpp Tue Nov 4
> 17:49:08 2014
> @@ -2549,23 +2549,31 @@ bool llvm:: isSafeToSpeculativelyExecute(
> 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<LoadInst>(Inst);
>
> Modified: llvm/trunk/test/Transforms/ LICM/speculate.ll
> URL: http://llvm.org/viewvc/llvm- project/llvm/trunk/test/
> Transforms/LICM/speculate.ll? rev=221318&r1=221317&r2=
> 221318&view=diff
> <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LICM/speculate.ll?rev=221318&r1=221317&r2=221318&view=diff>
> ============================== ==============================
> ==================
> --- llvm/trunk/test/Transforms/ LICM/speculate.ll (original)
> +++ llvm/trunk/test/Transforms/ LICM/speculate.ll Tue Nov 4
> 17:49:08 2014
> @@ -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:
> 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:
> ; 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:
> 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
>
>
> ______________________________ _________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
> <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list