Matt Turner <matts...@gmail.com> writes:
> Previously we made the basic block following an ENDIF instruction a
> successor of the basic blocks ending with IF and ELSE. The PRM says that
> IF and ELSE instructions jump *to* the ENDIF, rather than over it.
>
> This should be immaterial to dataflow analysis, except for if, break,
> endif sequences:
>
> START B1 <-B0 <-B9
> 0x00000100: cmp.g.f0(8) null g15<8,8,1>F g4<0,1,0>F
> 0x00000110: (+f0) if(8) 0 0 null 0x00000000UD
> END B1 ->B2 ->B4
> START B2 <-B1
> break
> 0x00000120: break(8) 0 0 null 0D
> END B2 ->B10
> START B3
> 0x00000130: endif(8) 2 null 0x00000002UD
> END B3 ->B4
>
> The ENDIF block would have no parents, so dataflow analysis would
> generate incorrect results, preventing copy propagation from eliminating
> some instructions.
>
> This patch changes the CFG to make ENDIF start rather than end basic
> blocks, so that it can be the jump target of the IF and ELSE
> instructions.
>
> It helps three programs (including two fs8/fs16 pairs) and hurts a
> single fs8/fs16 pair.
>
> total instructions in shared programs: 1561126 -> 1561066 (-0.00%)
> instructions in affected programs: 983 -> 923 (-6.10%)
>
> More importantly, it allows copy propagation to handle more cases.
> Disabling the register_coalesce() pass before this patch hurts 58
> programs, while afterward it only hurts 11 programs.
> ---
> src/mesa/drivers/dri/i965/brw_cfg.cpp | 46
> +++++++++++++++++++++++++----------
> 1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_cfg.cpp
> b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> index 548b458..83c3c34 100644
> --- a/src/mesa/drivers/dri/i965/brw_cfg.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_cfg.cpp
> @@ -133,10 +133,7 @@ cfg_t::create(void *parent_mem_ctx, exec_list
> *instructions)
>
> cur_if = cur;
> cur_else = NULL;
> - /* Set up the block just after the endif. Don't know when exactly
> - * it will start, yet.
> - */
> - cur_endif = new_block();
> + cur_endif = NULL;
>
> /* Set up our immediately following block, full of "then"
> * instructions.
> @@ -149,26 +146,49 @@ cfg_t::create(void *parent_mem_ctx, exec_list
> *instructions)
> break;
>
> case BRW_OPCODE_ELSE:
> - cur->add_successor(mem_ctx, cur_endif);
> + cur_else = cur;
>
> next = new_block();
> next->start = (backend_instruction *)inst->next;
> cur_if->add_successor(mem_ctx, next);
> - cur_else = next;
>
> set_next_block(next);
> break;
>
> case BRW_OPCODE_ENDIF: {
> - cur_endif->start = (backend_instruction *)inst->next;
> - cur->add_successor(mem_ctx, cur_endif);
> - set_next_block(cur_endif);
> + backend_instruction *prev_inst = (backend_instruction *)inst->prev;
> + switch (prev_inst->opcode) {
> + case BRW_OPCODE_IF:
> + case BRW_OPCODE_ELSE:
> + case BRW_OPCODE_WHILE:
> + case BRW_OPCODE_DO:
> + case BRW_OPCODE_BREAK:
> + case BRW_OPCODE_CONTINUE:
> + /* New block was just created; use it. */
> + cur_endif = cur;
> + break;Instead of the opcode switch for this test, couldn't you just do "if cur->start == inst"? Other than that, this series is: Reviewed-by: Eric Anholt <e...@anholt.net>
pgpsmYPhILiyM.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
