Hi Ken! Kenneth Graunke <kenn...@whitecape.org> writes:
> In theory we might have incorrectly NOP'd instructions that write the
> flag, but where that flag value isn't used, and yet the instruction
> either writes the accumulator or has side effects.
>
> I don't believe any such instructions exist, so this is mostly a
> code cleanup.
>
One example that occurred to me: The FB write opcode writes the flag
register (on Gen4-5 due to the dynamic AA payload hack), its flag result
is invariably dead (because the flag result is only ever used internally
to decide whether to send AA data or not), and has obvious side effects.
I believe the only reason why the code below wouldn't incorrectly nop
out FB writes is because of another, also somewhat concerning bug --
backend_instruction::flags_written() is completely unaware of FB writes
modifying the flag register on Gen4-5, which led to actual corruption in
my SIMD32 branch (I have a somewhat half-baked patch to fix it but I
think we should probably land this first).
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> index 8a0469a51b9..930dc733b45 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
> @@ -70,17 +70,11 @@ fs_visitor::dead_code_eliminate()
> }
> }
>
> - if (inst->dst.is_null() && inst->flags_written()) {
> - if (!(flag_live[0] & inst->flags_written())) {
> - inst->opcode = BRW_OPCODE_NOP;
> - progress = true;
> - }
> - }
> -
> if ((inst->opcode != BRW_OPCODE_IF &&
> inst->opcode != BRW_OPCODE_WHILE) &&
> inst->dst.is_null() &&
> !inst->has_side_effects() &&
> + !(flag_live[0] & inst->flags_written()) &&
I don't think this will have the intended effect unless you remove the
line below in addition, otherwise instructions that write a flag result
which happens to be dead will never get nop'ed out.
> !inst->flags_written() &&
> !inst->writes_accumulator) {
> inst->opcode = BRW_OPCODE_NOP;
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
