From 1a0bbc8a5c1e9cb9888ee8c2ba5f46752636e47f Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Sat, 16 Aug 2014 09:23:42 +0000 Subject: [PATCH] InstCombine: Fix a potential bug in 0 - (X sdiv C) -> (X sdiv -C) While *most* (X sdiv 1) operations will get caught by InstSimplify, it is still possible for a sdiv to appear in the worklist which hasn't been simplified yet. This means that it is possible for 0 - (X sdiv 1) to get transformed into (X sdiv -1); dividing by -1 can make the transform produce undef values instead of the proper result. Sorry for the lack of testcase, it's a bit problematic because it relies on the exact order of operations in the worklist. llvm-svn: 215818 --- llvm/include/llvm/IR/Constant.h | 3 +++ llvm/lib/IR/Constants.cpp | 22 +++++++++++++++++++ .../InstCombine/InstCombineAddSub.cpp | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/IR/Constant.h b/llvm/include/llvm/IR/Constant.h index 82ad9fc2f407..d9d163b23ecd 100644 --- a/llvm/include/llvm/IR/Constant.h +++ b/llvm/include/llvm/IR/Constant.h @@ -53,6 +53,9 @@ public: /// getNullValue. bool isNullValue() const; + /// \brief Returns true if the value is one. + bool isOneValue() const; + /// isAllOnesValue - Return true if this is the value that would be returned by /// getAllOnesValue. bool isAllOnesValue() const; diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp index b815936ac428..23845d8e3b8b 100644 --- a/llvm/lib/IR/Constants.cpp +++ b/llvm/lib/IR/Constants.cpp @@ -107,6 +107,28 @@ bool Constant::isAllOnesValue() const { return false; } +bool Constant::isOneValue() const { + // Check for 1 integers + if (const ConstantInt *CI = dyn_cast(this)) + return CI->isOne(); + + // Check for FP which are bitcasted from 1 integers + if (const ConstantFP *CFP = dyn_cast(this)) + return CFP->getValueAPF().bitcastToAPInt() == 1; + + // Check for constant vectors which are splats of 1 values. + if (const ConstantVector *CV = dyn_cast(this)) + if (Constant *Splat = CV->getSplatValue()) + return Splat->isOneValue(); + + // Check for constant vectors which are splats of 1 values. + if (const ConstantDataVector *CV = dyn_cast(this)) + if (Constant *Splat = CV->getSplatValue()) + return Splat->isOneValue(); + + return false; +} + bool Constant::isMinSignedValue() const { // Check for INT_MIN integers if (const ConstantInt *CI = dyn_cast(this)) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp index b18c10dd57b6..22cc34d861a5 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp @@ -1583,7 +1583,7 @@ Instruction *InstCombiner::visitSub(BinaryOperator &I) { // 0 - (X sdiv C) -> (X sdiv -C) provided the negation doesn't overflow. if (match(Op1, m_SDiv(m_Value(X), m_Constant(C))) && match(Op0, m_Zero()) && - !C->isMinSignedValue()) + !C->isMinSignedValue() && !C->isOneValue()) return BinaryOperator::CreateSDiv(X, ConstantExpr::getNeg(C)); // 0 - (X << Y) -> (-X << Y) when X is freely negatable.