[Mesa-dev] [PATCH 08/10] i965: Remove NOP insertion kludge in scheduler.

[Francisco Jerez]

Matt Turner <mattst88 at gmail.com> writes:

> Instead of removing every instruction in add_insts_from_block(), just
> move the instruction to its scheduled location. This is a step towards
> doing both bottom-up and top-down scheduling without conflicts.
>
> Note that this patch changes cycle counts for programs because it begins
> including control flow instructions in the estimates.
> ---
>  .../drivers/dri/i965/brw_schedule_instructions.cpp | 25 +++++-----------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> index 46b45a5..98fa5e3 100644
> --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> @@ -783,26 +783,13 @@ schedule_node::schedule_node(backend_instruction *inst,
>  void
>  instruction_scheduler::add_insts_from_block(bblock_t *block)
>  {
> -   /* Removing the last instruction from a basic block removes the block as
> -    * well, so put a NOP at the end to keep it alive.
> -    */
> -   if (!block->end()->is_control_flow()) {
> -      backend_instruction *nop = new(mem_ctx) backend_instruction();
> -      nop->opcode = BRW_OPCODE_NOP;
> -      block->end()->insert_after(block, nop);
> -   }
> -

Thanks for removing this hack, I had been meaning to do the same for a
while.  What was really bothering me about it is the fact that it
inserts a naked backend_instruction into the basic block's instruction
list, which everything else assumes contains elements of a concrete
instruction class, fs_inst or vec4_instruction depending on the
back-end -- So thanks a lot,

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

> -   foreach_inst_in_block_safe(backend_instruction, inst, block) {
> -      if (inst->opcode == BRW_OPCODE_NOP || inst->is_control_flow())
> -         continue;
> -
> +   foreach_inst_in_block(backend_instruction, inst, block) {
>        schedule_node *n = new(mem_ctx) schedule_node(inst, this);
>  
> -      this->instructions_to_schedule++;
> -
> -      inst->remove(block);
>        instructions.push_tail(n);
>     }
> +
> +   this->instructions_to_schedule = block->end_ip - block->start_ip + 1;
>  }
>  
>  /** Recursive computation of the delay member of a node. */
> @@ -1463,7 +1450,6 @@ void
>  instruction_scheduler::schedule_instructions(bblock_t *block)
>  {
>     const struct brw_device_info *devinfo = bs->devinfo;
> -   backend_instruction *inst = block->end();
>     time = 0;
>     if (!post_reg_alloc)
>        reg_pressure = reg_pressure_in[block->num];
> @@ -1482,7 +1468,8 @@ instruction_scheduler::schedule_instructions(bblock_t *block)
>        /* Schedule this instruction. */
>        assert(chosen);
>        chosen->remove();
> -      inst->insert_before(block, chosen->inst);
> +      chosen->inst->exec_node::remove();
> +      block->instructions.push_tail(chosen->inst);
>        instructions_to_schedule--;
>  
>        if (!post_reg_alloc) {
> @@ -1551,8 +1538,6 @@ instruction_scheduler::schedule_instructions(bblock_t *block)
>        }
>     }
>  
> -   if (block->end()->opcode == BRW_OPCODE_NOP)
> -      block->end()->remove(block);
>     assert(instructions_to_schedule == 0);
>  
>     block->cycle_count = time;
> -- 
> 2.4.10
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160313/68530fd6/attachment.sig>