On Monday, January 19, 2015 03:31:08 PM Matt Turner wrote:
> Otherwise we'll apply the conditional mod to only one of SIMD8
> instructions and trigger an assertion.
>
> NoDDClr/NoDDChk have the same problem but we never apply those to these
> instructions, so I'm leaving them for a later time.
> ---
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index ab848f1..f35da71 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1656,10 +1656,16 @@ fs_generator::generate_code(const cfg_t *cfg, int
> dispatch_width)
> brw_set_default_access_mode(p, BRW_ALIGN_16);
> if (dispatch_width == 16 && brw->gen < 8 && !brw->is_haswell) {
> brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> - brw_MAD(p, firsthalf(dst), firsthalf(src[0]), firsthalf(src[1]),
> firsthalf(src[2]));
> + brw_inst *f = brw_MAD(p, firsthalf(dst), firsthalf(src[0]),
> firsthalf(src[1]), firsthalf(src[2]));
> brw_set_default_compression_control(p, BRW_COMPRESSION_2NDHALF);
> - brw_MAD(p, sechalf(dst), sechalf(src[0]), sechalf(src[1]),
> sechalf(src[2]));
> + brw_inst *s = brw_MAD(p, sechalf(dst), sechalf(src[0]),
> sechalf(src[1]), sechalf(src[2]));
> brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> +
> + if (inst->conditional_mod) {
> + brw_inst_set_cond_modifier(brw, f, inst->conditional_mod);
> + brw_inst_set_cond_modifier(brw, s, inst->conditional_mod);
> + inst->conditional_mod = BRW_CONDITIONAL_NONE;
Having the generator mutate the incoming IR feels dirty to me. Honestly, it
should be const...we've never changed it until now.
I see what you're trying to accomplish - bypassing the assertion failure
about conditional_mod set with more than one instruction.
Maybe add a "bool multiple_instructions_allowed" flag, set it to false before
the switch, set it true here, and check it later to skip the assert? Seems
ugly, but not as bad as mutating the IR.
I think a better solution (after this series lands!) would be to generate two
MADs/LRPs at the fs_visitor level, and just emit a single instruction for each
at the generator level. We should have the infrastructure now and it'd let us
schedule them.
> + }
> } else {
> brw_MAD(p, dst, src[0], src[1], src[2]);
> }
> @@ -1671,10 +1677,16 @@ fs_generator::generate_code(const cfg_t *cfg, int
> dispatch_width)
> brw_set_default_access_mode(p, BRW_ALIGN_16);
> if (dispatch_width == 16 && brw->gen < 8 && !brw->is_haswell) {
> brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> - brw_LRP(p, firsthalf(dst), firsthalf(src[0]), firsthalf(src[1]),
> firsthalf(src[2]));
> + brw_inst *f = brw_LRP(p, firsthalf(dst), firsthalf(src[0]),
> firsthalf(src[1]), firsthalf(src[2]));
> brw_set_default_compression_control(p, BRW_COMPRESSION_2NDHALF);
> - brw_LRP(p, sechalf(dst), sechalf(src[0]), sechalf(src[1]),
> sechalf(src[2]));
> + brw_inst *s = brw_LRP(p, sechalf(dst), sechalf(src[0]),
> sechalf(src[1]), sechalf(src[2]));
> brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> +
> + if (inst->conditional_mod) {
> + brw_inst_set_cond_modifier(brw, f, inst->conditional_mod);
> + brw_inst_set_cond_modifier(brw, s, inst->conditional_mod);
> + inst->conditional_mod = BRW_CONDITIONAL_NONE;
> + }
> } else {
> brw_LRP(p, dst, src[0], src[1], src[2]);
> }
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
