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

[Francisco Jerez]

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).
-------------- 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/8e5c98ba/attachment-0001.sig>