On Thursday, December 04, 2014 03:37:17 PM Ben Widawsky wrote: > The GS has an interesting use for mul. It's essentially used as a fancy mov > (in > fact, I am not sure why a mov isn't used). The documentation in the function > has > a very good explanation from Paul on the mechanics. > > CHV has some quirks with regard to multiplication. While the documentation is > somewhat unclear, I've found that demoting the src1 operand in the GS mul > solves > all the problems. I'd ask that any potential reviewer ignore the other > instances > of mul for now (I have more patches), and simply make sure that what this > patch > does is correct.
(probably don't need review suggestions in the commit message; these normally go below the --- line) > This fixes around 2000 piglit tests on BSW. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84777 (with many dupes) > Cc: "10.4" <mesa-sta...@lists.freedesktop.org> Cc: "10.4 10.3 10.2" <mesa-sta...@lists.freedesktop.org> > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > > --- > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 5 ++++- > src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 3 ++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > index b353539..4f60797 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > @@ -534,8 +534,11 @@ vec4_generator::generate_gs_set_write_offset(struct > brw_reg dst, > brw_push_insn_state(p); > brw_set_default_access_mode(p, BRW_ALIGN_1); > brw_set_default_mask_control(p, BRW_MASK_DISABLE); > + assert(src1.file == BRW_IMMEDIATE_VALUE && > + src1.type == BRW_REGISTER_TYPE_UD && > + src1.dw1.ud <= SHRT_MAX); Don't you want USHRT_MAX? (I don't think it'll matter in practice.) > brw_MUL(p, suboffset(stride(dst, 2, 2, 1), 3), stride(src0, 8, 2, 4), > - src1); > + retype(src1, BRW_REGISTER_TYPE_UW)); Please add: assert(brw->gen >= 7); to this function, since UD * UW is illegal on Sandybridge: "When multiplying a DW and a W, the W has to be on src0, and the DW has to be on src1." Which is fine, since Sandybridge doesn't run this code. Gen7+ works like you expect, so I think this is a good change to make. > brw_set_default_access_mode(p, BRW_ALIGN_16); > brw_pop_insn_state(p); > } > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > index db0e6cc..2d7ae5a 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp > @@ -388,7 +388,8 @@ vec4_gs_visitor::emit_control_data_bits() > */ > src_reg per_slot_offset(this, glsl_type::uint_type); > emit(SHR(dst_reg(per_slot_offset), dword_index, 2u)); > - emit(GS_OPCODE_SET_WRITE_OFFSET, mrf_reg, per_slot_offset, 1u); > + emit(GS_OPCODE_SET_WRITE_OFFSET, mrf_reg, per_slot_offset, > + src_reg(1u)); This hunk looks unnecessary (please drop it). > } > > if (urb_write_flags & BRW_URB_WRITE_USE_CHANNEL_MASKS) { With those changes, Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
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