The simplifier drops unused LetStmts even if the RHS has side effects.

[benoitsteiner]

I passed a Target object with NoAssert turned on to compile_to_lowered_stmt on a pipeline of mine. Here's what I get:

module name=117_0_t, target=x86-64-linux-avx-avx2-avx512-avx512_skylake-f16c-fma-no_asserts-no_bounds_query-sse41 buffer b0 = {...} assert(((uint64)reinterpret(((halide_buffer_t ))b0.buffer) != (uint64)0), halide_error_buffer_argument_is_null("b0")) let b0.min.0 = _halide_buffer_get_min(((halide_buffer_t ))b0.buffer, 0) let b0.extent.0 = _halide_buffer_get_extent(((halide_buffer_t ))b0.buffer, 0) let b0.stride.0 = _halide_buffer_get_stride(((halide_buffer_t ))b0.buffer, 0) assert((b0.stride.0 == 1), 0) assert((b0.min.0 == 0), 0) assert((b0.extent.0 == 1000), 0)

I didn't expect to see these assertion. Is there another way to get rid of them ? I have a pipeline with hundreds of inputs, and pages after pages of assertions that validate the shapes of the inputs.

[abadams]

They're still there in the IR so that the simplifier will exploit them, but they should get compiled to no-ops.

That said, I'm not convinced that they do. Single asserts get skipped, but I think blocks of them might not be due to the change that blocked asserts together. Will investigate to be sure: https://github.com/halide/Halide/blob/master/src/CodeGen_LLVM.cpp#L4214

[abadams]

They'll still show up in the IR, but they'll be compiled to no-ops. I ran into trouble when outright deleting them to do with the way the simplifier strips dead lets.

[steven-johnson]

They'll still show up in the IR, but they'll be compiled to no-ops. I ran into trouble when outright deleting them to do with the way the simplifier strips dead lets.

Would it be sensible to add a comment to the generated Stmt to the effect of "/ Ignore These /"?

[abadams]

I'd like to actually strip them, because the reason they're still there is to do with us not accounting for side-effects properly when stripping lets. That was going to be a larger change though, so I fixed the bug more directly. I'll reopen the bug.

[abadams]

The specific issue is when we have:

let foo = side_effect() in
assert(foo = 0)

If we strip the assert, the let is dead, and also gets stripped, removing the side effect. This is because the let statement visitor in the simplifier doesn't check for side-effects before duplicating or stripping things. Neither do our arithmetic cancellation rules. Adding that check involves adding a flag to the simplifier to track properties of subexpressions (e.g. has-side-effect, has-load). We should then do things like not simplify x - x -> 0 when x has a side effect. More simply, we should just not do any algebraic mutations on expressions with side effects.

[abadams]

I looked at this again, and I think the best thing to do is to continue to strip them at codegen time. There's no point stripping them earlier. However, we should still fix the issue with dropping side-effecty let statements.