Skip to content

Commit 0e64045

Browse files
Handle many more intrinsics in Bounds.cpp
This addresses many (but not all) of the `signed integer overflow` issues we're seeing in Google due to #7814 -- a lot of the issues seems to be in code that uses intrinsics that had no handling in value bounds checking, so the bounds were naively large and overflowed. - Most of the intrinsics from FindIntrinsics.h weren't handled; now they all are (most by lowering to other IR, though the halving_add variants were modeled directly because the bitwise ops don't mesh well) - strict_float() is just a pass-through - round() is a best guess (basically, if bounds exist, expand by one as a worst-case) There are definitely others we should handle here... trunc/floor/ceil probably?
1 parent 3a1dffe commit 0e64045

File tree

1 file changed

+42
-9
lines changed

1 file changed

+42
-9
lines changed

Diff for: src/Bounds.cpp

+42-9
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ using std::string;
4141
using std::vector;
4242

4343
namespace {
44+
45+
Expr widen(Expr a) {
46+
Type result_type = a.type().widen();
47+
return Cast::make(result_type, std::move(a));
48+
}
49+
50+
Expr narrow(Expr a) {
51+
Type result_type = a.type().narrow();
52+
return Cast::make(result_type, std::move(a));
53+
}
54+
4455
int static_sign(const Expr &x) {
4556
if (is_positive_const(x)) {
4657
return 1;
@@ -56,6 +67,7 @@ int static_sign(const Expr &x) {
5667
}
5768
return 0;
5869
}
70+
5971
} // anonymous namespace
6072

6173
const FuncValueBounds &empty_func_value_bounds() {
@@ -1276,6 +1288,18 @@ class Bounds : public IRVisitor {
12761288
} else if (op->is_intrinsic(Call::return_second)) {
12771289
internal_assert(op->args.size() == 2);
12781290
interval = arg_bounds.get(1);
1291+
} else if (op->is_intrinsic(Call::strict_float)) {
1292+
internal_assert(op->args.size() == 1);
1293+
interval = arg_bounds.get(0);
1294+
} else if (op->is_intrinsic(Call::round)) {
1295+
internal_assert(op->args.size() == 1);
1296+
interval = arg_bounds.get(0);
1297+
if (interval.has_lower_bound()) {
1298+
interval.min -= 1.f;
1299+
}
1300+
if (interval.has_upper_bound()) {
1301+
interval.max += 1.f;
1302+
}
12791303
} else if (op->is_intrinsic(Call::if_then_else)) {
12801304
internal_assert(op->args.size() == 2 || op->args.size() == 3);
12811305
// Probably more conservative than necessary
@@ -1517,15 +1541,24 @@ class Bounds : public IRVisitor {
15171541
result.include(arg_bounds.get(i));
15181542
}
15191543
interval = result;
1520-
} else if (op->is_intrinsic(Call::widen_right_add)) {
1521-
Expr add = Add::make(op->args[0], cast(op->args[0].type(), op->args[1]));
1522-
add.accept(this);
1523-
} else if (op->is_intrinsic(Call::widen_right_sub)) {
1524-
Expr sub = Sub::make(op->args[0], cast(op->args[0].type(), op->args[1]));
1525-
sub.accept(this);
1526-
} else if (op->is_intrinsic(Call::widen_right_mul)) {
1527-
Expr mul = Mul::make(op->args[0], cast(op->args[0].type(), op->args[1]));
1528-
mul.accept(this);
1544+
} else if (op->is_intrinsic(Call::halving_add)) {
1545+
// lower_halving_add() uses bitwise tricks that are hard to reason
1546+
// about; let's do this instead:
1547+
Expr e = narrow((widen(op->args[0]) + widen(op->args[1])) / 2);
1548+
e.accept(this);
1549+
} else if (op->is_intrinsic(Call::rounding_halving_add)) {
1550+
// lower_rounding_halving_add() uses bitwise tricks that are hard to reason
1551+
// about; let's do this instead:
1552+
Expr e = narrow((widen(op->args[0]) + widen(op->args[1]) + 1) / 2);
1553+
e.accept(this);
1554+
} else if (op->call_type == Call::PureIntrinsic) {
1555+
Expr e = lower_intrinsic(op);
1556+
if (e.defined()) {
1557+
e.accept(this);
1558+
} else {
1559+
// Just use the bounds of the type
1560+
bounds_of_type(t);
1561+
}
15291562
} else if (op->call_type == Call::Halide) {
15301563
bounds_of_func(op->name, op->value_index, op->type);
15311564
} else {

0 commit comments

Comments
 (0)