Francisco Jerez <curroje...@riseup.net> writes: > Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes: > >> From: "Juan A. Suarez Romero" <jasua...@igalia.com> >> >> When converting a DF to F, we set dst stride to 2, to fulfil alignment >> restrictions. >> >> But in IVB/BYT, this is not necessary, as each DF conversion already >> writes 2 F, the first one the real value, and the second one a 0. That >> is, IVB/BYT already set stride = 2 implicitly, so we must set it to 1 >> explicitly to avoid ending up with stride = 4. >> >> v2: >> - Fix typo (Matt) >> --- >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp >> index 45881e3ec95..487f2e90224 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp >> @@ -1629,6 +1629,16 @@ fs_generator::generate_code(const cfg_t *cfg, int >> dispatch_width) >> inst->src[i].type != BRW_REGISTER_TYPE_UD || >> !inst->src[i].negate); >> } >> + /* When converting from DF->F, we set destination's stride as 2 as an >> + * alignment requirement. But in IVB/BYT, each DF implicitly writes 2 >> F, >> + * being the first one the converted value. So we don't need to >> + * explicitly set stride 2, but 1. >> + */ >> + if (devinfo->gen == 7 && !devinfo->is_haswell && >> + type_sz(inst->src[0].type) > type_sz(inst->dst.type)) { > > This should be exec_type_size(inst) rather than > type_sz(inst->src[0].type). > >> + assert(inst->dst.stride == 2 || inst->dst.stride == 1); >> + inst->dst.stride = 1; >> + } > > This is modifying the IR, please don't. > > Also I don't think the above has the same semantics as a destination > region with stride 2... AFAIUI IVB will just write garbage into the odd > channels when the destination type is narrower than a DF which is really > not what a strided move is supposed to do. If that's the case it would > probably be safer to add a new F64TO32 virtual opcode for type > conversions and assert(inst->dst.stride == 1) here... >
It seems like my intuition matches the simulator's behavior, unfortunately... >> dst = brw_reg_from_fs_reg(compiler->devinfo, inst, >> &inst->dst, compressed); >> >> -- >> 2.11.0 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev