On Tue, 2017-01-10 at 14:01 -0800, Francisco Jerez wrote: > Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: > > > On Mon, 2017-01-09 at 16:18 -0800, Francisco Jerez wrote: > > > Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: > > > > > > > From: Iago Toral Quiroga <ito...@igalia.com> > > > > > > > > It seems to use 1 channel por DF, just like later hardware. The > > > > docs say things > > > > like: > > > > > > > > "Each DF operand uses a pair of channels and all masking and > > > > swizzling > > > > should be adjusted appropriately." > > > > > > > > "In Align16, all regioning parameters must use the syntax of a > > > > pair > > > > of packed > > > > floats, including channel selects and channel enables." > > > > > > > > In these cases, masking and channel select/enables seem to > > > > refere > > > > only to > > > > Align16's swizzle and writemasks respectively, not to execution > > > > masking. > > > > > > > > In any case, this means that execution masking controls and > > > > execsize go > > > > in different units and we need to adjust the assertion on the > > > > relation > > > > between the two accordingly. > > > > --- > > > > src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 6 +++++- > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp > > > > b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp > > > > index 90ee7c1..ac2d8ad 100644 > > > > --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp > > > > +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp > > > > @@ -1646,7 +1646,11 @@ fs_generator::generate_code(const cfg_t > > > > *cfg, int dispatch_width) > > > > brw_set_default_exec_size(p, cvt(inst->exec_size) - 1); > > > > > > > > assert(inst->force_writemask_all || inst->exec_size >= > > > > 4); > > > > - assert(inst->force_writemask_all || inst->group % inst- > > > > > exec_size == 0); > > > > > > > > + assert(inst->force_writemask_all || > > > > + ((devinfo->gen != 7 || devinfo->is_haswell) && > > > > + inst->group % inst->exec_size == 0) || > > > > + ((devinfo->gen == 7 && !devinfo->is_haswell) && > > > > + (2 * inst->group) % inst->exec_size == 0)); > > > > > > NAK, exec size doubling happens at the codegen level so the IR > > > group > > > and > > > exec_size controls should still be expressed in the same units. > > > > > > > Actually the patch is right because the exec size doubling already > > happened at this point, so this is fixing the difference in units. > > > > It may match the expectations of PATCH 3 from this series, but at the > same time it breaks the expectations of the rest of the back-end IR > infrastructure. E.g. regs_read() will now return bogus results from > the > point you whacked the execution size of DF instructions. I don't > want > to have to think which portion of the public IR interface still > behaves > sensibly and which portion will give me garbage values when I use it > From the generator. >
Right. > To make the matter scarier this happens from the back-end code > generator > which isn't even supposed to modify its input in a semantics- > preserving > way (it's supposed to emit machine code matching the IR given as > input). > PATCH 3 causes it to destroy its input (try invoking the code > generator > a second time on the same shader and you'll get garbage as > result...). > I see. Thanks for the explanation! > > One alternative to this patch is to move the exec_size doubling > > code to > > just after this assert, so we can discard this patch. > > > > What do you think? > > > > Please let's not modify the IR from the codegen pass [this goes as > modification request for PATCH 3 too]. Just have the generator emit > machine code with twice the IR execution size for double-precision > instructions on IVB. > OK, I'll do this change. Sam > > Sam > > > > > > assert(inst->base_mrf + inst->mlen <= > > > > BRW_MAX_MRF(devinfo- > > > > > gen)); > > > > > > > > assert(inst->mlen <= BRW_MAX_MSG_LENGTH); > > > > > > > > -- > > > > 2.9.3 > > > > > > > > _______________________________________________ > > > > mesa-dev mailing list > > > > mesa-dev@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
