[Fixed]Dangerous optimization in simplify

xqdan · October 15, 2018, 6:31am

dmlc/HalideIR/blob/2f3ecdfdedf3efa7e45a3945dca63a25856c4674/src/arithmetic/Simplify.cpp#L844


           equal(min_a->b, max_b->a)) {
    // min(x, y) + max(y, x) -> x + y
    expr = mutate(min_a->a + min_a->b);
} else if (no_overflow(op->type) &&
           div_a &&
           add_a_a &&
           const_int(add_a_a->b, &ia) &&
           const_int(div_a->b, &ib) && ib &&
           const_int(b, &ic)) {
    // ((a + ia) / ib + ic) -> (a + (ia + ib*ic)) / ib
    expr = mutate((add_a_a->a + IntImm::make(op->type, ia + ib*ic)) / div_a->b);
} else if (mul_a &&
           mul_b &&
           equal(mul_a->a, mul_b->a)) {
    // Pull out common factors a*x + b*x
    expr = mutate(mul_a->a * (mul_a->b + mul_b->b));
} else if (mul_a &&
           mul_b &&
           equal(mul_a->b, mul_b->a)) {
    expr = mutate(mul_a->b * (mul_a->a + mul_b->b));
} else if (mul_a &&

for this case, ((a + ia) / ib + ic) -> (a + (ia + ib*ic)) / ib

when ic is negative, ic=-1, ib=2 and a + ia = 1
before optimization,
1/ 2 - 1 = -1
after optimization
（1 - 1 × 2） / 2 = 0
comes out different result, if hardware round up differently for 1/2 and -1/2

for our hardware, round up like, 1/2=0 -1/2=0

@tqchen if needed I can send a PR

tqchen · October 9, 2018, 4:10pm

Sure, we can send a PR to disable this optimization. The main problem is that Halide used a different semantics of div and mod(which rounds down) for negative integers as well.

xqdan · October 10, 2018, 3:35am

thanks, will PR soon