On Tuesday, February 03, 2015 10:17:36 PM Matt Turner wrote:
> Prevents piglit regressions from the next patch.
> ---
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 37
> +++++++++++++++++++++++++-
> 1 file changed, 36 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 77d4908..8cd36f8 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1734,7 +1734,42 @@ fs_generator::generate_code(const cfg_t *cfg, int
> dispatch_width)
> brw_F16TO32(p, dst, src[0]);
> break;
> case BRW_OPCODE_CMP:
> - brw_CMP(p, dst, inst->conditional_mod, src[0], src[1]);
> + /* The Ivybridge/BayTrail WaCMPInstFlagDepClearedEarly workaround
> says
> + * that when the destination is a GRF that the dependency-clear bit
> on
> + * the flag register is cleared early.
> + *
> + * Suggested workarounds are to disable coissuing CMP instructions
> + * or to split CMP(16) instructions into two CMP(8) instructions.
> + *
> + * We choose to split into CMP(8) instructions since disabling
> + * coissuing would affect CMP instructions not otherwise affected by
> + * the errata.
> + */
> + if (dispatch_width == 16 && brw->gen == 7 && !brw->is_haswell) {
> + if (dst.file == BRW_GENERAL_REGISTER_FILE) {
> + brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> + brw_CMP(p, firsthalf(dst), inst->conditional_mod,
> + firsthalf(src[0]), firsthalf(src[1]));
> + brw_set_default_compression_control(p,
> BRW_COMPRESSION_2NDHALF);
> + brw_CMP(p, sechalf(dst), inst->conditional_mod,
> + sechalf(src[0]), sechalf(src[1]));
> + brw_set_default_compression_control(p,
> BRW_COMPRESSION_COMPRESSED);
> +
> + multiple_instructions_emitted = true;
> + } else if (dst.file == BRW_ARCHITECTURE_REGISTER_FILE) {
> + /* For unknown reasons, the aforementioned workaround is not
> + * sufficient. Overriding the type when the destination is the
> + * null register is necessary but not sufficient by itself.
> + */
> + assert(dst.nr == BRW_ARF_NULL);
> + dst.type = BRW_REGISTER_TYPE_D;
> + brw_CMP(p, dst, inst->conditional_mod, src[0], src[1]);
> + } else {
> + unreachable("not reached");
> + }
> + } else {
> + brw_CMP(p, dst, inst->conditional_mod, src[0], src[1]);
> + }
> break;
> case BRW_OPCODE_SEL:
> brw_SEL(p, dst, src[0], src[1]);
> These three patches seem reasonable to me. Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> I do wonder, though - overriding the destination type to D effectively disables coissuing for a single instruction. I know the documentation claims that source types = F and destination types = D isn't legal, but we've been doing it for ages with no apparent issues. This might be a less expensive workaround. Then again, if it were that easy, presumably the other driver team would've done it that way...
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
