[MLIR] MathExtras: support dividing large numbers

[arjunp]

Summary

The current implementations of ceilDiv and floorDiv involve multiplying
the operands. This results in an overflow when the operands are large, whereas
one might assume that dividing numbers should not result in overflow unless the
divisor is -1.

[bondhugula]

mlir/include/mlir/Support/MathExtras.h

27

You can double-check if parentheses around the disjunction are needed. Perhaps better with extra parentheses around?

PS: '||' does precede the ternary operator but they are pretty close!

[arjunp]

Add extra parentheses

[arjunp]

mlir/include/mlir/Support/MathExtras.h

27

Dropping the parentheses around the disjunction will produce warnings. I added extra parenthesis around the whole condition.

[arjunp]

Similar patch was already landed. (D116096)

mlir/include/mlir/Support/MathExtras.h

 namespace mlir {
 
 /// Returns the result of MLIR's ceildiv operation on constants. The RHS is
 /// expected to be non-zero.
 inline int64_t ceilDiv(int64_t lhs, int64_t rhs) {
   assert(rhs != 0);
   // C/C++'s integer division rounds towards 0.
   int64_t x = (rhs > 0) ? -1 : 1;
-  return (lhs * rhs > 0) ? ((lhs + x) / rhs) + 1 : -(-lhs / rhs);
+  return (lhs > 0 && rhs > 0) || (lhs < 0 && rhs < 0) ? ((lhs + x) / rhs) + 1
+                                                      : -(-lhs / rhs);
 }
 
 /// Returns the result of MLIR's floordiv operation on constants. The RHS is
 /// expected to be non-zero.
 inline int64_t floorDiv(int64_t lhs, int64_t rhs) {
   assert(rhs != 0);
   // C/C++'s integer division rounds towards 0.
   int64_t x = (rhs < 0) ? 1 : -1;
-  return (lhs * rhs < 0) ? -((-lhs + x) / rhs) - 1 : lhs / rhs;
+  return (lhs > 0 && rhs < 0) || (lhs < 0 && rhs > 0) ? -((-lhs + x) / rhs) - 1
+                                                      : lhs / rhs;
 }
 
 /// Returns MLIR's mod operation on constants. MLIR's mod operation yields the
 /// remainder of the Euclidean division of 'lhs' by 'rhs', and is therefore not
 /// C's % operator.  The RHS is always expected to be positive, and the result
 /// is always non-negative.
 inline int64_t mod(int64_t lhs, int64_t rhs) {
   assert(rhs >= 1);

mlir/unittests/Support/MathExtrasTest.cpp

 using namespace mlir;
 using ::testing::Eq;
 
 TEST(MathExtrasTest, CeilDivTest) {
   EXPECT_THAT(ceilDiv(14, 3), Eq(5));
   EXPECT_THAT(ceilDiv(14, -3), Eq(-4));
   EXPECT_THAT(ceilDiv(-14, -3), Eq(5));
   EXPECT_THAT(ceilDiv(-14, 3), Eq(-4));
+  EXPECT_THAT(ceilDiv(1ll << 60, 1ll << 60), Eq(1));
 }
 
 TEST(MathExtrasTest, FloorDivTest) {
   EXPECT_THAT(floorDiv(14, 3), Eq(4));
   EXPECT_THAT(floorDiv(14, -3), Eq(-5));
   EXPECT_THAT(floorDiv(-14, -3), Eq(4));
   EXPECT_THAT(floorDiv(-14, 3), Eq(-5));
+  EXPECT_THAT(floorDiv(1ll << 60, 1ll << 60), Eq(1));
 }