nir_opt_preamble isn't sound
Consider the shader:
if (pointer_foo_is_valid) {
bar = *foo;
} else {
bar = 0;
}
If *foo
is dereferenced with load_global_constant
, then nir_opt_preamble
will happily rewrite this to:
uniform = *foo;
---
if (pointer_foo_is_valid) {
bar = uniform;
} else {
bar = 0;
}
and for kicks peephole select will kick in to get a nice clean
uniform = *foo;
---
bar = pointer_foo_is_valid ? uniform : 0;
The problem is that we now dereference foo
even when it isn't valid! Sure, the result is disregarded in the end -- so the logical behaviour of the program is unchanged -- but if foo
points to unmapped memory then executing the preamble will fault even though the original program is perfectly valid.
This isn't a theoretical issue, the arb_texture_buffer_object-max-size
piglit hits this on Asahi with our wacky buffer texture lowering.
There are a few options here:
-
Accept the unsoundness and only use
nir_opt_preamble
on platforms where MMU faults are silently ignored. We'll probably end up doing this in production for Asahi at some point (the kernel controls whether faults are suppressed), but it doesn't seem like a great solution. -
Refuse to hoist loads out of conditional control flow.
-
When hoisting loads out of conditional control flow, replicate the control flow. This would work for the particular case above (and the texture buffer piglit) but it seems really tricky in the general case especially when loops are thrown into the mix.
Cc @cwabbott0
Also cc @robclark -- unless freedreno/turnip is suppressing faults (option 1) I think this issue might exists there too.