ASSERT and __builtin_unreachable

[Marc Glisse]

On Tue, 3 Sep 2019, Niels Möller wrote:

> I think we discussed this earlier, but I don't recall the conclusion, if
> any. Would it make sense to use ASSERT to guide the compiler, and define
> it like
>
> #define ASSERT(expr) do { if (!(expr)) __builtin_unreachable(); } while (0)
>
> (unless building with --enable-assert)?
>
> The idea is to tell the compiler to assume that the asserted expression
> is true, and not care what the behavior of the generated code is in case
> it nevertheless happens to be false at run time.
>
> As far as I understand, this should aid optimization in some (rare?)
> cases. Drawbacks are small: we'd need a configure test for
> __builtin_unreachable, and there are a few places with code like
>
>   ASSERT_CODE (char *bp_orig = bp - bytes);
>   ...
>   ASSERT (bp > bp_orig);
>
> that needs the current definition of ASSERT and have to be updated in
> one way or the other.

That seems fine for small assertions, say ASSERT(x>0), but it seems bad 
for more expensive ones, because it becomes an ASSERT_ALWAYS. In ASSERT 
(refmpn_mul_1c (prod, rp, size, divisor, carry) == carry_orig) we may end 
up calling refmpn_mul_1c needlessly, same in ASSERT (mpn_mod_1 (up, n, d) 
== 0). So we may need to introduce ASSERT_EXPENSIVE at the same time.

There are other drawbacks, it may hinder inlining because the tests make 
the functions temporarily bigger than they should be (though 
__builtin_constant_p is much worse in that respect), and it may limit 
other optimizations, but we can probably ignore that unless we notice a 
regression.

-- 
Marc Glisse