[Mesa-dev] [PATCH 01/12] i965/fs: Define logical texture sampling opcodes.

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> Kenneth Graunke <kenneth at whitecape.org> writes:
>
>> On Saturday, July 18, 2015 05:34:47 PM Francisco Jerez wrote:
>>> Each logical variant is largely equivalent to the original opcode but
>>> instead of taking a single payload source it expects the arguments
>>> separately as individual sources, like:
>>> 
>>>  tex_logical dst, coordinates, shadow_c, lod, lod2,
>>>                   sample_index, mcs, sampler, offset,
>>>                   num_coordinate_components, num_grad_components
>>> 
>>> This patch defines the opcodes and usual instruction boilerplate,
>>> including a placeholder lowering function provided mostly as
>>> documentation for their source registers.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_defines.h  | 12 +++++
>>>  src/mesa/drivers/dri/i965/brw_fs.cpp     | 92 ++++++++++++++++++++++++++++++++
>>>  src/mesa/drivers/dri/i965/brw_shader.cpp | 25 +++++++++
>>>  3 files changed, 129 insertions(+)
>>> 
>>> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
>>> index 9099676..193fcbe 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_defines.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>>> @@ -890,17 +890,29 @@ enum opcode {
>>>     SHADER_OPCODE_COS,
>>>  
>>>     SHADER_OPCODE_TEX,
>>> +   SHADER_OPCODE_TEX_LOGICAL,
>>>     SHADER_OPCODE_TXD,
>>> +   SHADER_OPCODE_TXD_LOGICAL,
>>>     SHADER_OPCODE_TXF,
>>> +   SHADER_OPCODE_TXF_LOGICAL,
>>>     SHADER_OPCODE_TXL,
>>> +   SHADER_OPCODE_TXL_LOGICAL,
>>>     SHADER_OPCODE_TXS,
>>> +   SHADER_OPCODE_TXS_LOGICAL,
>>>     FS_OPCODE_TXB,
>>> +   FS_OPCODE_TXB_LOGICAL,
>>>     SHADER_OPCODE_TXF_CMS,
>>> +   SHADER_OPCODE_TXF_CMS_LOGICAL,
>>>     SHADER_OPCODE_TXF_UMS,
>>> +   SHADER_OPCODE_TXF_UMS_LOGICAL,
>>>     SHADER_OPCODE_TXF_MCS,
>>> +   SHADER_OPCODE_TXF_MCS_LOGICAL,
>>>     SHADER_OPCODE_LOD,
>>> +   SHADER_OPCODE_LOD_LOGICAL,
>>>     SHADER_OPCODE_TG4,
>>> +   SHADER_OPCODE_TG4_LOGICAL,
>>>     SHADER_OPCODE_TG4_OFFSET,
>>> +   SHADER_OPCODE_TG4_OFFSET_LOGICAL,
>>>  
>>>     /**
>>>      * Combines multiple sources of size 1 into a larger virtual GRF.
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> index 503d4d8..6afb9fe 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>> @@ -711,6 +711,31 @@ fs_inst::regs_read(int arg) const
>>>           components = src[6].fixed_hw_reg.dw1.ud;
>>>        break;
>>>  
>>> +   case SHADER_OPCODE_TEX_LOGICAL:
>>> +   case SHADER_OPCODE_TXD_LOGICAL:
>>> +   case SHADER_OPCODE_TXF_LOGICAL:
>>> +   case SHADER_OPCODE_TXL_LOGICAL:
>>> +   case SHADER_OPCODE_TXS_LOGICAL:
>>> +   case FS_OPCODE_TXB_LOGICAL:
>>> +   case SHADER_OPCODE_TXF_CMS_LOGICAL:
>>> +   case SHADER_OPCODE_TXF_UMS_LOGICAL:
>>> +   case SHADER_OPCODE_TXF_MCS_LOGICAL:
>>> +   case SHADER_OPCODE_LOD_LOGICAL:
>>> +   case SHADER_OPCODE_TG4_LOGICAL:
>>> +   case SHADER_OPCODE_TG4_OFFSET_LOGICAL:
>>> +      assert(src[8].file == IMM && src[9].file == IMM);
>>> +      /* Texture coordinates. */
>>> +      if (arg == 0)
>>> +         components = src[8].fixed_hw_reg.dw1.ud;
>>> +      /* Texture derivatives/LOD. */
>>> +      else if (arg == 2 || arg == 3)
>>> +         components = (opcode == SHADER_OPCODE_TXD_LOGICAL ?
>>> +                       src[9].fixed_hw_reg.dw1.ud : 1);
>>> +      /* Texture offset. */
>>> +      else if (arg == 7)
>>> +         components = 2;
>>
>> Is this right?  textureOffset() on sampler1D/2D/3D has an offset
>> parameter with type int/ivec2/ivec3.  2 might not be enough...
>
> Yeah, two is enough because this source is only actually read by the
> TG4_OFFSET instruction, which expects a 2D offset value, other
> instructions still pass the offset through the old-style
> backend_instruction::offset field.
>
> I guess it would be nice to eventually get rid of
> backend_instruction::offset and other highly opcode-specific fields
> defined in the top-level instruction structure which seem to have little
> benefit over plain source registers and are most of the time unused and
> a waste of memory.
>
> When we do that two components will indeed not be enough.  How about we
> redefine src[9] to determine the number of components of both the
> texture derivatives and offset?  AFAIK when they're both present they're
> always the same value (src[8] may be different though for array textures
> and such).

Meh...  Not sure that's a good idea, it introduces some (currently
useless) complication to fs_visitor::emit_texture() and
::nir_emit_texture() (see attachment).  We may just leave it alone and
change it when src[7] is allowed to take anything else than two
components, either by redefining src[9] or by adding a new immediate
source to specify the number of offset components.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: i965-tex_logical-explicit_offset_components.patch
Type: text/x-diff
Size: 5222 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150724/a998cce9/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150724/a998cce9/attachment-0001.sig>