[Mesa-dev] [PATCH 09/12] i965/fs: Hook up SIMD lowering to handle texturing opcodes of unsupported width.

[Kenneth Graunke]

On Saturday, July 18, 2015 05:34:55 PM Francisco Jerez wrote:
> This should match the set of cases in which we currently call fail()
> or no16() from the emit_texture_*() methods and the ones in which
> emit_texture_gen4() enables the SIMD16 workaround.
> 
> Hint for reviewers: It's not a big deal if I happen to have missed
> some case here, it will just lead to an assertion failure down the
> road which is easily fixable, however being stricter than necessary
> won't cause any visible breakage, it would just decrease performance
> silently due to the unnecessary message splitting, so feel free to
> double-check that all cases listed here already cause a SIMD8/16
> fall-back with the current texturing code -- You may want to skip over
> the Gen5-6 cases though if you don't have pencil and paper at hand.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 043d9e9..f291202 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3918,6 +3918,33 @@ get_lowered_simd_width(const struct brw_device_info *devinfo,
>        /* Dual-source FB writes are unsupported in SIMD16 mode. */
>        return (inst->src[1].file != BAD_FILE ? 8 : inst->exec_size);
>  
> +   case SHADER_OPCODE_TXD_LOGICAL:
> +      /* TXD is unsupported in SIMD16 mode. */
> +      return 8;
> +
> +   case SHADER_OPCODE_TG4_OFFSET_LOGICAL: {
> +      /* gather4_po_c is unsupported in SIMD16 mode. */
> +      const fs_reg &shadow_c = inst->src[1];
> +      return (shadow_c.file != BAD_FILE ? 8 : inst->exec_size);
> +   }
> +   case SHADER_OPCODE_TXL_LOGICAL:
> +   case FS_OPCODE_TXB_LOGICAL: {
> +      /* Gen4 doesn't have SIMD8 non-shadow-compare bias/LOD instructions, and
> +       * Gen4-6 don't support TXL and TXB with shadow comparison in SIMD16
> +       * mode.
> +       */

I agree with Jason here, I think this would be a lot easier to read if
we separated the gen == 4 and gen <= 6 cases into two if statements.
(I do like pure expressions in general, though!)

While you're doing that, it would be great to say

/* Gen4-6 can't support TXL and TXB with shadow comparison in SIMD16
 * mode because the message exceeds the maximum length of 11.
 */

(it wasn't obvious to me why that was disallowed, at first)

Other than my concern about regs_read() for offset in patch 1,
everything looks great to me.  With that sorted out, this series is:

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks Curro!
-------------- 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/20150723/a607870c/attachment.sig>