On Tuesday, May 27, 2014 06:47:41 PM Matt Turner wrote: > Clean up with with register_coalesce()/dead_code_eliminate(). > --- > src/mesa/drivers/dri/i965/brw_fs.cpp | 42 ++++++++++++++++++++++++++++++++++++ > src/mesa/drivers/dri/i965/brw_fs.h | 1 + > 2 files changed, 43 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp > index 0856b6b..c0af6d0 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > @@ -2574,6 +2574,43 @@ fs_visitor::lower_uniform_pull_constant_loads() > } > } > > +bool > +fs_visitor::lower_load_payload() > +{ > + bool progress = false; > + > + foreach_list_safe(node, &instructions) { > + fs_inst *inst = (fs_inst *)node; > + > + if (inst->opcode == SHADER_OPCODE_LOAD_PAYLOAD) { > + fs_reg dst = inst->dst; > + > + /* The generator creates the message header if present, which is in > + * the first register of the message payload. > + */ > + if (!inst->header_present) {
I don't think this will work like you want. In your emit_texture_gen7 patch, when the texturing operation needs a header, you set src[0] to reg_undef (BAD_FILE). But you don't set header_present on the LOAD_PAYLOAD instruction. Here, inst is the LOAD_PAYLOAD operation...so you're checking header_present on...not what you thought. It sure looks like this will generate MOV(dst, reg_undef) which is bad. I think you actually should just go with your original version of this patch: http://article.gmane.org/gmane.comp.video.mesa3d.devel/76592 AFAICT, the semantics of LOAD_PAYLOAD are: /** * Combines multiple sources of size 1 into a larger virtual GRF. * For example, parameters for a send-from-GRF message. Or, updating * channels of a size 4 VGRF used to store vec4s such as texturing results. * * This will be lowered into MOVs from each source to consecutive reg_offsets * of the destination VGRF. * * src[0] may be BAD_FILE. If so, the lowering pass skips emitting the MOV, * but still reserves the first channel of the destination VGRF. This can be * used to reserve space for, say, a message header set up by the generators. */ I'd like a comment like that to appear above the SHADER_OPCODE_LOAD_PAYLOAD opcode in brw_defines.h. Feel free to improve the wording. > + inst->insert_before(MOV(dst, inst->src[0])); > + } else { > + assert(inst->src[0].file == BAD_FILE); > + } > + dst.reg_offset++; > + > + for (int i = 1; i < inst->sources; i++) { > + inst->insert_before(MOV(dst, inst->src[i])); > + dst.reg_offset++; > + } > + > + inst->remove(); > + progress = true; > + } > + } > + > + if (progress) > + invalidate_live_intervals(); > + > + return progress; > +} > + > void > fs_visitor::dump_instructions() > { > @@ -3071,6 +3108,11 @@ fs_visitor::run() > progress = OPT(compute_to_mrf) || progress; > } while (progress); > > + if (lower_load_payload()) { > + register_coalesce(); > + dead_code_eliminate(); > + } > + > lower_uniform_pull_constant_loads(); > > assign_curb_setup(); > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h > index d0e459c..2b60945 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.h > +++ b/src/mesa/drivers/dri/i965/brw_fs.h > @@ -391,6 +391,7 @@ public: > void fail(const char *msg, ...); > void no16(const char *msg, ...); > void lower_uniform_pull_constant_loads(); > + bool lower_load_payload(); > > void push_force_uncompressed(); > void pop_force_uncompressed(); >
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev