On Tue, Jan 20, 2015 at 12:32 AM, Kenneth Graunke <kenn...@whitecape.org> wrote: > 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.
Sure. > 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. I really don't think adding that complication is a good idea. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev