[llvm] 7cbb107 - [Attributor][FIX] Validate the type for AAValueConstantRange as needed

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2020-02-14T17:22:40-06:00
New Revision: 7cbb107feb45a58f34199d2da4b84f8670c1c831

URL: https://github.com/llvm/llvm-project/commit/7cbb107feb45a58f34199d2da4b84f8670c1c831
DIFF: https://github.com/llvm/llvm-project/commit/7cbb107feb45a58f34199d2da4b84f8670c1c831.diff

LOG: [Attributor][FIX] Validate the type for AAValueConstantRange as needed

Due to the genericValueTraversal we might visit values for which we did
not create an AAValueConstantRange object, e.g., as they are behind a
PHI or select or call with `returned` argument. As a consequence we need
to validate the types as we are about to query AAValueConstantRange for
operands.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/range.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 0b985db161d0..57babcb94ff5 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -6266,15 +6266,8 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
       return;
     }
 
-    if (auto *I = dyn_cast<Instruction>(&V))
-      if (isa<BinaryOperator>(I) || isa<CmpInst>(I)) {
-        Value *LHS = I->getOperand(0);
-        Value *RHS = I->getOperand(1);
-
-        if (LHS->getType()->isIntegerTy() && RHS->getType()->isIntegerTy())
-          return;
-      }
-
+    if (isa<BinaryOperator>(&V) || isa<CmpInst>(&V) || isa<CastInst>(&V))
+      return;
     // If it is a load instruction with range metadata, use it.
     if (LoadInst *LI = dyn_cast<LoadInst>(&V))
       if (auto *RangeMD = LI->getMetadata(LLVMContext::MD_range)) {
@@ -6282,12 +6275,6 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
         return;
       }
 
-    // We handle casts in the updateImpl.
-    // TODO: Allow non integers as well.
-    if (CastInst *CI = dyn_cast<CastInst>(&V))
-      if (CI->getOperand(0)->getType()->isIntegerTy())
-        return;
-
     // We can work with PHI and select instruction as we traverse their operands
     // during update.
     if (isa<SelectInst>(V) || isa<PHINode>(V))
@@ -6306,6 +6293,9 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
       SmallVectorImpl<const AAValueConstantRange *> &QuerriedAAs) {
     Value *LHS = BinOp->getOperand(0);
     Value *RHS = BinOp->getOperand(1);
+    // TODO: Allow non integers as well.
+    if (!LHS->getType()->isIntegerTy() || !RHS->getType()->isIntegerTy())
+      return false;
 
     auto &LHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*LHS));
@@ -6332,7 +6322,8 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
     assert(CastI->getNumOperands() == 1 && "Expected cast to be unary!");
     // TODO: Allow non integers as well.
     Value &OpV = *CastI->getOperand(0);
-    assert(OpV.getType()->isIntegerTy() && "Expected integer cast");
+    if (!OpV.getType()->isIntegerTy())
+      return false;
 
     auto &OpAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(OpV));
@@ -6348,6 +6339,9 @@ struct AAValueConstantRangeFloating : AAValueConstantRangeImpl {
                    SmallVectorImpl<const AAValueConstantRange *> &QuerriedAAs) {
     Value *LHS = CmpI->getOperand(0);
     Value *RHS = CmpI->getOperand(1);
+    // TODO: Allow non integers as well.
+    if (!LHS->getType()->isIntegerTy() || !RHS->getType()->isIntegerTy())
+      return false;
 
     auto &LHSAA =
         A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(*LHS));

diff  --git a/llvm/test/Transforms/Attributor/range.ll b/llvm/test/Transforms/Attributor/range.ll
index a3f2b2e4af45..f05f55e0c4f2 100644
--- a/llvm/test/Transforms/Attributor/range.ll
+++ b/llvm/test/Transforms/Attributor/range.ll
@@ -1131,6 +1131,37 @@ entry:
 
 ; }
 
+define i1 @f_fcmp(float %a, float %b) {
+  %r = fcmp uge float %a, %b
+  %s = select i1 %r, i1 %r, i1 0
+  ret i1 %s
+}
+define i1 @d_fcmp(double %a, double %b) {
+  %r = fcmp oeq double %a, %b
+  %s = select i1 %r, i1 %r, i1 0
+  ret i1 %s
+}
+define i1 @dp_icmp(double* %a, double* %b) {
+  %r = icmp sge double* %a, %b
+  %s = select i1 %r, i1 %r, i1 0
+  ret i1 %s
+}
+define i1 @ip_icmp(i8* %a, i8* %b) {
+  %r = icmp ult i8* %a, %b
+  %s = select i1 %r, i1 %r, i1 0
+  ret i1 %s
+}
+define i1 @fcmp_caller(float %fa, float %fb, double %da, double %db, double* %dpa, double* %dpb, i8* %ipa, i8* %ipb) {
+  %r1 = call i1 @f_fcmp(float %fa, float %fb)
+  %r2 = call i1 @d_fcmp(double %da, double %db)
+  %r3 = call i1 @dp_icmp(double* %dpa, double* %dpb)
+  %r4 = call i1 @ip_icmp(i8* %ipa, i8* %ipb)
+  %o1 = or i1 %r1, %r2
+  %o2 = or i1 %r3, %r4
+  %o3 = or i1 %o1, %o2
+  ret i1 %o3
+}
+
 !0 = !{i32 0, i32 10}
 !1 = !{i32 10, i32 100}
 ; CHECK: !0 = !{i32 0, i32 10}