Jason Ekstrand <ja...@jlekstrand.net> writes: > On Jul 29, 2015 3:12 AM, "Francisco Jerez" <curroje...@riseup.net> wrote: >> >> 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). > > Then let's change the commit message to say that the execution controls > don't matter. With that, you can have my R-B on the last two. > Alright, I'll fix that.
> Is Tue SIMD stuff unblocked now?
Yeah, I already pushed most of it, thanks :)
> --Jason
>
>> >> ---
>> >> 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
