[llvm] 52b4490 - [IR] fix crash in Constant::isElementWiseEqual() with FP types

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2020-01-16T16:49:16-05:00
New Revision: 52b44902d059e68c6b5553c1d043f768c516064a

URL: https://github.com/llvm/llvm-project/commit/52b44902d059e68c6b5553c1d043f768c516064a
DIFF: https://github.com/llvm/llvm-project/commit/52b44902d059e68c6b5553c1d043f768c516064a.diff

LOG: [IR] fix crash in Constant::isElementWiseEqual() with FP types

We lifted this code from InstCombine for general usage in:
rL369842
...but it's not safe as-is. There are no existing users that can
trigger this bug, but I discovered it via crashing several
regression tests when trying to use it for select folding in
InstSimplify.

ICmp requires (vector) integer types, so give up on anything that's
not integer or FP (pointers and ?) then bitcast the constants
before trying the match. That matches the definition of "equal or
undef" that I was looking for. If someone wants an FP-aware version
of equality (deal with NaN, -0.0), that could be a different mode
or different function.

Differential Revision: https://reviews.llvm.org/D72784

Added: 
    

Modified: 
    llvm/lib/IR/Constants.cpp
    llvm/unittests/IR/ConstantsTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 054375aab6c3..93d406fa2eb0 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -286,12 +286,17 @@ bool Constant::isElementWiseEqual(Value *Y) const {
   if (!isa<Constant>(Y) || !Ty->isVectorTy() || Ty != Y->getType())
     return false;
 
+  // TODO: Compare pointer constants?
+  if (!(Ty->getVectorElementType()->isIntegerTy() ||
+        Ty->getVectorElementType()->isFloatingPointTy()))
+    return false;
+
   // They may still be identical element-wise (if they have `undef`s).
-  // FIXME: This crashes on FP vector constants.
-  return match(ConstantExpr::getICmp(ICmpInst::Predicate::ICMP_EQ,
-                                     const_cast<Constant *>(this),
-                                     cast<Constant>(Y)),
-               m_One());
+  // Bitcast to integer to allow exact bitwise comparison for all types.
+  Type *IntTy = VectorType::getInteger(cast<VectorType>(Ty));
+  Constant *C0 = ConstantExpr::getBitCast(const_cast<Constant *>(this), IntTy);
+  Constant *C1 = ConstantExpr::getBitCast(cast<Constant>(Y), IntTy);
+  return match(ConstantExpr::getICmp(ICmpInst::ICMP_EQ, C0, C1), m_One());
 }
 
 bool Constant::containsUndefElement() const {

diff  --git a/llvm/unittests/IR/ConstantsTest.cpp b/llvm/unittests/IR/ConstantsTest.cpp
index 8a3336210e22..305555fb741f 100644
--- a/llvm/unittests/IR/ConstantsTest.cpp
+++ b/llvm/unittests/IR/ConstantsTest.cpp
@@ -586,7 +586,8 @@ TEST(ConstantsTest, FoldGlobalVariablePtr) {
 }
 
 // Check that undefined elements in vector constants are matched
-// correctly for both integer and floating-point types.
+// correctly for both integer and floating-point types. Just don't
+// crash on vectors of pointers (could be handled?).
 
 TEST(ConstantsTest, isElementWiseEqual) {
   LLVMContext Context;
@@ -607,7 +608,6 @@ TEST(ConstantsTest, isElementWiseEqual) {
   EXPECT_FALSE(C12U1->isElementWiseEqual(C12U2));
   EXPECT_FALSE(C12U21->isElementWiseEqual(C12U2));
 
-/* FIXME: This will crash.
   Type *FltTy = Type::getFloatTy(Context);
   Constant *CFU = UndefValue::get(FltTy);
   Constant *CF1 = ConstantFP::get(FltTy, 1.0);
@@ -621,7 +621,19 @@ TEST(ConstantsTest, isElementWiseEqual) {
   EXPECT_TRUE(CF12U1->isElementWiseEqual(CF1211));
   EXPECT_FALSE(CF12U2->isElementWiseEqual(CF12U1));
   EXPECT_FALSE(CF12U1->isElementWiseEqual(CF12U2));
-*/
+
+  PointerType *PtrTy = Type::getInt8PtrTy(Context);
+  Constant *CPU = UndefValue::get(PtrTy);
+  Constant *CP0 = ConstantPointerNull::get(PtrTy);
+
+  Constant *CP0000 = ConstantVector::get({CP0, CP0, CP0, CP0});
+  Constant *CP00U0 = ConstantVector::get({CP0, CP0, CPU, CP0});
+  Constant *CP00U = ConstantVector::get({CP0, CP0, CPU});
+
+  EXPECT_FALSE(CP0000->isElementWiseEqual(CP00U0));
+  EXPECT_FALSE(CP00U0->isElementWiseEqual(CP0000));
+  EXPECT_FALSE(CP0000->isElementWiseEqual(CP00U));
+  EXPECT_FALSE(CP00U->isElementWiseEqual(CP00U0));
 }
 
 }  // end anonymous namespace