On Fri, 2015-09-18 at 09:09 -0700, Kenneth Graunke wrote:
> On Friday, September 18, 2015 10:08:52 AM Iago Toral Quiroga wrote:
> > Until now we only used MRFs 1..15 for regular SEND messages, so the
> > message length could not possibly exceed the maximum size. Now that
> > we allow to use MRF registers 1..23 in gen6, we need to be careful
> > not to build messages that can go beyond the limit. That could occur,
> > specifically, when building URB write messages, which we may need to
> > split in chunks due to their size. Previously we would simply go and
> > create a new message when we reached MRF 13 (since 13..15 were
> > reserved for spilling), now we also want to check the size of the
> > message explicitly.
> >
> > Besides adding that condition to split URB write messages properly,
> > this patch also adds asserts in the generator. Notice that
> > brw_inst_set_mlen already asserts for this, but asserting in the
> > generators is easy and can make debugging easier in some cases.
> > ---
> > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 1 +
> > src/mesa/drivers/dri/i965/brw_inst.h | 3 +++
> > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 1 +
> > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 5 +++--
> > 4 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > index 6cbd42f..c65084d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> > @@ -1561,6 +1561,7 @@ fs_generator::generate_code(const cfg_t *cfg, int
> > dispatch_width)
> > brw_set_default_exec_size(p, cvt(inst->exec_size) - 1);
> >
> > assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo->gen));
> > + assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
> >
> > switch (inst->exec_size) {
> > case 1:
> > diff --git a/src/mesa/drivers/dri/i965/brw_inst.h
> > b/src/mesa/drivers/dri/i965/brw_inst.h
> > index 46eff1d..c5132ba 100644
> > --- a/src/mesa/drivers/dri/i965/brw_inst.h
> > +++ b/src/mesa/drivers/dri/i965/brw_inst.h
> > @@ -39,6 +39,9 @@
> > extern "C" {
> > #endif
> >
> > +/** Maximum SEND message length */
> > +#define BRW_MAX_MSG_LENGTH 15
> > +
> > /* brw_context.h has a forward declaration of brw_inst, so name the
> > struct. */
> > typedef struct brw_inst {
> > uint64_t data[2];
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> > b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> > index d5943d2..05f2044 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> > @@ -1136,6 +1136,7 @@ vec4_generator::generate_code(const cfg_t *cfg)
> > brw_set_default_acc_write_control(p, inst->writes_accumulator);
> >
> > assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo->gen));
> > + assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
> >
> > unsigned pre_emit_nr_insn = p->nr_insn;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > index 7f06050..514ccd6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > @@ -3320,9 +3320,10 @@ vec4_visitor::emit_vertex()
> > prog_data->vue_map.slot_to_varying[slot]);
> >
> > /* If this was max_usable_mrf, we can't fit anything more into
> > this
> > - * URB WRITE.
> > + * URB WRITE. Same thing if we reached the maximum length
> > available.
> > */
> > - if (mrf > max_usable_mrf) {
> > + if (mrf > max_usable_mrf ||
> > + align_interleaved_urb_mlen(devinfo, mrf - base_mrf + 1) >
> > BRW_MAX_MSG_LENGTH) {
> > slot++;
> > break;
> > }
> >
>
> Doesn't this hunk need to go before patch 2? It seems like otherwise
> we'll be emitting URB write messages that are too long until patch 5.
Yeah, you're right. I'll fix that before pushing.
> With that fixed, the series is:
> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>
> Thanks for doing this, Iago!
Thanks for reviewing it! ;)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
