[ConstantRange] Fix the early out in ConstantRange::multiply for positive numbers to really do what the comment says

[craig.topper]

Summary

r271020 added an early out to skip the signed multiply portion of ConstantRange::multiply. The comment says we don't need to do signed multiply if the range is only positive numbers, but the implemented check only ensures that the start of the range is positive. It doesn't look at the end of the range.

This patch checks the end of the range instead. Because Upper is one more than the end we have to see if its positive or if its one past the last positive number.

[pete]

Good catch! LGTM.

[craig.topper]

Committed in r302717, but forgot to include the Differential Revision line

lib/IR/ConstantRange.cpp

ConstantRange::multiply(const ConstantRange &Other) const {
   ConstantRange Result_zext = ConstantRange(this_min * Other_min,
                                             this_max * Other_max + 1);
   ConstantRange UR = Result_zext.truncate(getBitWidth());
 
   // If the unsigned range doesn't wrap, and isn't negative then it's a range
   // from one positive number to another which is as good as we can generate.
   // In this case, skip the extra work of generating signed ranges which aren't
   // going to be better than this range.
-  if (!UR.isWrappedSet() && UR.getLower().isNonNegative())
+  if (!UR.isWrappedSet() &&
+      (UR.getUpper().isNonNegative() || UR.getUpper().isMinSignedValue()))
     return UR;
 
   // Now the signed range. Because we could be dealing with negative numbers
   // here, the lower bound is the smallest of the cartesian product of the
   // lower and upper ranges; for example:
   //   [-1,4) * [-2,3) = min(-1*-2, -1*2, 3*-2, 3*2) = -6.
   // Similarly for the upper bound, swapping min for max.
 

unittests/IR/ConstantRangeTest.cpp

EXPECT_EQ(ConstantRange(APInt(8, 254), APInt(8, 0)).multiply(
             ConstantRange(APInt(8, 250), APInt(8, 9)));
   EXPECT_EQ(ConstantRange(APInt(8, 254), APInt(8, 255)).multiply(
               ConstantRange(APInt(8, 2), APInt(8, 4))),
             ConstantRange(APInt(8, 250), APInt(8, 253)));
 
   // TODO: This should be return [-2, 0]
   EXPECT_EQ(ConstantRange(APInt(8, -2)).multiply(
               ConstantRange(APInt(8, 0), APInt(8, 2))),
-            ConstantRange(APInt(8, 0), APInt(8, 255)));
+            ConstantRange(APInt(8, -2), APInt(8, 1)));
 }
 
 TEST_F(ConstantRangeTest, UMax) {
   EXPECT_EQ(Full.umax(Full), Full);
   EXPECT_EQ(Full.umax(Empty), Empty);
   EXPECT_EQ(Full.umax(Some), ConstantRange(APInt(16, 0xa), APInt(16, 0)));
   EXPECT_EQ(Full.umax(Wrap), Full);
   EXPECT_EQ(Full.umax(Some), ConstantRange(APInt(16, 0xa), APInt(16, 0)));