Matt Turner <matts...@gmail.com> writes: > On Thu, Jun 4, 2015 at 9:05 AM, Francisco Jerez <curroje...@riseup.net> wrote: >> --- >> src/mesa/drivers/dri/i965/brw_fs.cpp | 9 ++--- >> src/mesa/drivers/dri/i965/brw_fs.h | 3 +- >> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 57 >> ++++++++++++++-------------- >> 3 files changed, 35 insertions(+), 34 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >> b/src/mesa/drivers/dri/i965/brw_fs.cpp >> index e55998c..4c1aa60 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> @@ -2918,13 +2918,12 @@ fs_visitor::emit_repclear_shader() >> int base_mrf = 1; >> int color_mrf = base_mrf + 2; >> >> - fs_inst *mov = emit(MOV(vec4(brw_message_reg(color_mrf)), >> - fs_reg(UNIFORM, 0, BRW_REGISTER_TYPE_F))); >> - mov->force_writemask_all = true; >> + fs_inst *mov = bld.exec_all().MOV(vec4(brw_message_reg(color_mrf)), >> + fs_reg(UNIFORM, 0, >> BRW_REGISTER_TYPE_F)); >> >> fs_inst *write; >> if (key->nr_color_regions == 1) { >> - write = emit(FS_OPCODE_REP_FB_WRITE); >> + write = bld.emit(FS_OPCODE_REP_FB_WRITE); >> write->saturate = key->clamp_fragment_color; >> write->base_mrf = color_mrf; >> write->target = 0; >> @@ -2933,7 +2932,7 @@ fs_visitor::emit_repclear_shader() >> } else { >> assume(key->nr_color_regions > 0); >> for (int i = 0; i < key->nr_color_regions; ++i) { >> - write = emit(FS_OPCODE_REP_FB_WRITE); >> + write = bld.emit(FS_OPCODE_REP_FB_WRITE); >> write->saturate = key->clamp_fragment_color; >> write->base_mrf = base_mrf; >> write->target = i; >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h >> b/src/mesa/drivers/dri/i965/brw_fs.h >> index de37298..571d411 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.h >> +++ b/src/mesa/drivers/dri/i965/brw_fs.h >> @@ -330,7 +330,8 @@ public: >> void setup_color_payload(fs_reg *dst, fs_reg color, unsigned components, >> unsigned exec_size, bool use_2nd_half); >> void emit_alpha_test(); >> - fs_inst *emit_single_fb_write(fs_reg color1, fs_reg color2, >> + fs_inst *emit_single_fb_write(const brw::fs_builder &bld, >> + fs_reg color1, fs_reg color2, >> fs_reg src0_alpha, unsigned components, >> unsigned exec_size, bool use_2nd_half = >> false); >> void emit_fb_writes(); >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> index 5d6d6d6..98429fa 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp >> @@ -1296,12 +1296,12 @@ fs_visitor::emit_dummy_fs() >> /* Everyone's favorite color. */ >> const float color[4] = { 1.0, 0.0, 1.0, 0.0 }; >> for (int i = 0; i < 4; i++) { >> - emit(MOV(fs_reg(MRF, 2 + i * reg_width, BRW_REGISTER_TYPE_F, >> - dispatch_width), fs_reg(color[i]))); >> + bld.MOV(fs_reg(MRF, 2 + i * reg_width, BRW_REGISTER_TYPE_F, >> + dispatch_width), fs_reg(color[i])); >> } >> >> fs_inst *write; >> - write = emit(FS_OPCODE_FB_WRITE); >> + write = bld.emit(FS_OPCODE_FB_WRITE); >> write->eot = true; >> if (devinfo->gen >= 6) { >> write->base_mrf = 2; >> @@ -1479,7 +1479,7 @@ fs_visitor::setup_color_payload(fs_reg *dst, fs_reg >> color, unsigned components, >> fs_reg tmp = vgrf(glsl_type::vec4_type); >> assert(color.type == BRW_REGISTER_TYPE_F); >> for (unsigned i = 0; i < components; i++) { >> - inst = emit(MOV(offset(tmp, i), offset(color, i))); >> + inst = bld.MOV(offset(tmp, i), offset(color, i)); >> inst->saturate = true; >> } >> color = tmp; >> @@ -1550,15 +1550,14 @@ fs_visitor::emit_alpha_test() >> } >> >> fs_inst * >> -fs_visitor::emit_single_fb_write(fs_reg color0, fs_reg color1, >> +fs_visitor::emit_single_fb_write(const fs_builder &bld, >> + fs_reg color0, fs_reg color1, >> fs_reg src0_alpha, unsigned components, >> unsigned exec_size, bool use_2nd_half) >> { >> assert(stage == MESA_SHADER_FRAGMENT); >> brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data; >> brw_wm_prog_key *key = (brw_wm_prog_key*) this->key; >> - >> - this->current_annotation = "FB write header"; >> int header_size = 2, payload_header_size; >> >> /* We can potentially have a message length of up to 15, so we have to >> set >> @@ -1589,22 +1588,23 @@ fs_visitor::emit_single_fb_write(fs_reg color0, >> fs_reg color1, >> >> if (payload.aa_dest_stencil_reg) { >> sources[length] = fs_reg(GRF, alloc.allocate(1)); >> - emit(MOV(sources[length], >> - fs_reg(brw_vec8_grf(payload.aa_dest_stencil_reg, 0)))); >> + bld.exec_all().annotate("FB write stencil/AA alpha") >> + .MOV(sources[length], >> + fs_reg(brw_vec8_grf(payload.aa_dest_stencil_reg, 0))); >> length++; >> } >> >> prog_data->uses_omask = >> prog->OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK); >> if (prog_data->uses_omask) { >> - this->current_annotation = "FB write oMask"; >> assert(this->sample_mask.file != BAD_FILE); >> /* Hand over gl_SampleMask. Only lower 16 bits are relevant. Since >> * it's unsinged single words, one vgrf is always 16-wide. >> */ >> sources[length] = fs_reg(GRF, alloc.allocate(1), >> BRW_REGISTER_TYPE_UW, 16); >> - emit(FS_OPCODE_SET_OMASK, sources[length], this->sample_mask); >> + bld.exec_all().annotate("FB write oMask") >> + .emit(FS_OPCODE_SET_OMASK, sources[length], this->sample_mask); >> length++; >> } >> >> @@ -1665,20 +1665,21 @@ fs_visitor::emit_single_fb_write(fs_reg color0, >> fs_reg color1, >> if (payload.dest_depth_reg) >> sources[length++] = fs_reg(brw_vec8_grf(payload.dest_depth_reg, 0)); >> >> + const fs_builder ubld = bld.group(exec_size, use_2nd_half); >> fs_inst *load; >> fs_inst *write; >> if (devinfo->gen >= 7) { >> /* Send from the GRF */ >> fs_reg payload = fs_reg(GRF, -1, BRW_REGISTER_TYPE_F, exec_size); >> - load = emit(LOAD_PAYLOAD(payload, sources, length, >> payload_header_size)); >> + load = ubld.LOAD_PAYLOAD(payload, sources, length, >> payload_header_size); > > I don't see that the LOAD_PAYLOAD had force_sechalf set before. Same > comment elsewhere in this patch. > > What's going on? If this is a functional change... you need to write a > commit message!
It's neither an accident nor a functional change. As I mentioned in the commit message of PATCH 14, the builder requires you to be explicit about the subset of channel enables you want to affect your instructions (or call exec_all() if you don't want any at all) if you are planning to emit any instruction of non-native SIMD width. Otherwise the assertion in fs_builder::emit() fires. This is not a functional change because we don't emit the FB writes under non-uniform control flow (we can't because of EOT), and because the kill-pixel mask is always sent as a header rather than via predication on HSW and up for dual-source writes, so always using the first quarter was probably okay, by pure luck. I can repeat basically the same justification as in PATCH 14 in the commit message of this patch, if you'd like it to be more clear.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev