On Tue, Sep 23, 2014 at 12:50 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > On Thu, Aug 28, 2014 at 8:10 PM, Matt Turner <matts...@gmail.com> wrote: >> >> The array was previously indexed in units of brw_compact_inst (8-bytes), >> but before compaction all instructions are uncompacted, so every odd >> element was unused. >> --- >> src/mesa/drivers/dri/i965/brw_eu_compact.c | 59 >> +++++++++++++++++++----------- >> 1 file changed, 37 insertions(+), 22 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c >> b/src/mesa/drivers/dri/i965/brw_eu_compact.c >> index c291f96..acce663 100644 >> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c >> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c >> @@ -1033,20 +1033,32 @@ static void >> update_uip_jip(struct brw_context *brw, brw_inst *insn, >> int this_old_ip, int *compacted_counts) >> { >> - int scale = brw->gen >= 8 ? sizeof(brw_compact_inst) : 1; >> - >> - int32_t jip = brw_inst_jip(brw, insn) / scale; >> - jip -= compacted_between(this_old_ip, this_old_ip + jip, >> compacted_counts); >> - brw_inst_set_jip(brw, insn, jip * scale); >> + /* JIP and UIP are in units of: >> + * - bytes on Gen8+; and >> + * - compacted instructions on Gen6+. >> + */ >> + int32_t jip = brw_inst_jip(brw, insn); >> + int32_t jip_compacted = jip / (brw->gen >= 8 ? >> sizeof(brw_compact_inst) : 1); >> + int32_t jip_uncompacted = jip / (brw->gen >= 8 ? sizeof(brw_inst) : >> 2); >> + jip_compacted -= compacted_between(this_old_ip, >> + this_old_ip + jip_uncompacted, >> + compacted_counts); > > > Is this correct on gen >= 8? It seems as if you would be missing a factor > of sizeof(brw_compact_inst). In general, this and the hunk below it seem > very fragile. Can we do these calculations without the gen >= 8 specific > stuff and then do the factor of 8 at the end?
Not in a way I can see if we want to use the same array indexing on all platforms (doing otherwise seems awful). The code is complicated because the JIP and UIP are stored in different units on Gen8+. I think immediately converting into common units of compacted-instructions and uncompacted-instructions is the simplest thing to do. Unless I've misunderstood your feedback? Also, the line below your comment converts the units of compacted-instructions back into the hardware format: >> >> + brw_inst_set_jip(brw, insn, >> + jip_compacted * (brw->gen >= 8 ? sizeof(brw_inst) : >> 1)); _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev