On Wed, Sep 23, 2015 at 5:25 PM, Jason Ekstrand <[email protected]> wrote: > On Wed, Sep 23, 2015 at 5:11 PM, Kristian Høgsberg Kristensen > <[email protected]> wrote: >> When we assign hw regs to attributes, we don't incorporate the stride >> and subreg_offset from the fs_reg. It's rarely used, but the integer >> multiplication lowering uses unusual stride and subreg_offset >> combination breaks when one source is an attribute. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91970 >> Cc: "11.0" <[email protected]> >> Signed-off-by: Kristian Høgsberg Kristensen <[email protected]> >> --- >> src/mesa/drivers/dri/i965/brw_fs.cpp | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >> b/src/mesa/drivers/dri/i965/brw_fs.cpp >> index 225a312..618bbd2 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> @@ -1564,7 +1564,10 @@ fs_visitor::assign_vs_urb_setup() >> >> inst->src[i].file = HW_REG; >> inst->src[i].fixed_hw_reg = >> - retype(brw_vec8_grf(grf, 0), inst->src[i].type); >> + stride(byte_offset(retype(brw_vec8_grf(grf, 0), >> inst->src[i].type), >> + inst->src[i].subreg_offset), >> + inst->exec_size * inst->src[i].stride, >> + inst->exec_size, inst->src[i].stride); > > I made this comment to Kristian today in the office, but I figure I'll > also put it on the list: > > I'm not 100% convinced that I like having subreg offsets and strides > respected on the ATTR. We do respect subreg offset on the UNIFORM > file so I guess there's some precedent for it. If we didn't, another > option would be to simply emit a MOV to a grf during lowering if we > ever detect ATTR. Given that this is only on CHV and probably not hit > that often, I think that would be my minor preference. That said, > this patch does work.
IIRC you discussed uniforms in this context with Curro on-list and he wasn't in favor of it, but I may be misremembering. I don't understand what benefit would be gained by ignoring subreg offset (and I don't see that it's stated anywhere). _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
