[Mesa-dev] [PATCH 09/16] i965/fs: Apply conditional mod specially to split MAD/LRP.

[Kenneth Graunke]

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]);
>  	 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150120/0a2b646e/attachment.sig>