Jason Ekstrand <ja...@jlekstrand.net> writes: > On Tue, Jul 28, 2015 at 1:23 AM, Francisco Jerez <curroje...@riseup.net> > wrote: >> Instead of relying on the default one. This shouldn't lead to any >> functional changes because DEP_RESOLVE_MOV overrides the execution >> controls of the instruction anyway. > > Actually, DEP_RESOLVE_MOV calls half() on the builder which doesn't > make any sense. I think the pre-builder intention, based on the > comment, was force_uncompressed and to use a SIMD8 instruction so that > it adds as few deps as possible. I'm not sure what it's actually > doing now. In any case, saying that it overrides everything isn't > right. > Pre-builder it was doing the same it's doing now -- Emit an 8-wide instruction regardless of the dispatch width of the shader. fs_builder::half(0) is an alias for ::group(8, 0) which simply selects the first 8-channel group of the channel enables and will cause the instruction to be uncompressed during code generation.
You're right that it doesn't override execution controls of the instruction other than exec_size, but that's because it doesn't need to. force_writemask_all and force_sechalf are fully irrelevant for what the emitted MOV is intended to do: stall the pipeline until the instruction that was writing the GRF provided as argument retires, which is going to happen regardless of the EMask of the MOV instruction (even if it's zero it should cause a stall because pre-IVB hardware didn't implement shoot-down of instructions with zero EMask). >> --- >> src/mesa/drivers/dri/i965/brw_fs.cpp | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >> b/src/mesa/drivers/dri/i965/brw_fs.cpp >> index 71d372c..8bc9372 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> @@ -2827,7 +2827,8 @@ >> fs_visitor::insert_gen4_pre_send_dependency_workarounds(bblock_t *block, >> if (block->start() == scan_inst) { >> for (int i = 0; i < write_len; i++) { >> if (needs_dep[i]) >> - DEP_RESOLVE_MOV(bld.at(block, inst), first_write_grf + i); >> + DEP_RESOLVE_MOV(fs_builder(this, block, inst), >> + first_write_grf + i); >> } >> return; >> } >> @@ -2843,7 +2844,7 @@ >> fs_visitor::insert_gen4_pre_send_dependency_workarounds(bblock_t *block, >> if (reg >= first_write_grf && >> reg < first_write_grf + write_len && >> needs_dep[reg - first_write_grf]) { >> - DEP_RESOLVE_MOV(bld.at(block, inst), reg); >> + DEP_RESOLVE_MOV(fs_builder(this, block, inst), reg); >> needs_dep[reg - first_write_grf] = false; >> if (scan_inst->exec_size == 16) >> needs_dep[reg - first_write_grf + 1] = false; >> @@ -2890,7 +2891,8 @@ >> fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, >> fs_ins >> if (block->end() == scan_inst) { >> for (int i = 0; i < write_len; i++) { >> if (needs_dep[i]) >> - DEP_RESOLVE_MOV(bld.at(block, scan_inst), first_write_grf + >> i); >> + DEP_RESOLVE_MOV(fs_builder(this, block, scan_inst), >> + first_write_grf + i); >> } >> return; >> } >> @@ -2905,7 +2907,8 @@ >> fs_visitor::insert_gen4_post_send_dependency_workarounds(bblock_t *block, >> fs_ins >> scan_inst->dst.reg >= first_write_grf && >> scan_inst->dst.reg < first_write_grf + write_len && >> needs_dep[scan_inst->dst.reg - first_write_grf]) { >> - DEP_RESOLVE_MOV(bld.at(block, scan_inst), scan_inst->dst.reg); >> + DEP_RESOLVE_MOV(fs_builder(this, block, scan_inst), >> + scan_inst->dst.reg); >> needs_dep[scan_inst->dst.reg - first_write_grf] = false; >> } >> >> -- >> 2.4.6 >>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev