Francisco Jerez <curroje...@riseup.net> writes:
> Kenneth Graunke <kenn...@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.
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
index 06dc835..c7abb64 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -920,7 +920,8 @@ enum opcode {
* Source 6: [required] Texture sampler.
* Source 7: [optional] Texel offset.
* Source 8: [required] Number of coordinate components (as UD immediate).
- * Source 9: [required] Number derivative components (as UD immediate).
+ * Source 9: [required] Number of derivative or offset components (as UD
+ * immediate).
*/
SHADER_OPCODE_TEX,
SHADER_OPCODE_TEX_LOGICAL,
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 169b070..5b8a03d 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -703,7 +703,7 @@ fs_inst::components_read(unsigned i) const
return src[9].fixed_hw_reg.dw1.ud;
/* Texture offset. */
else if (i == 7)
- return 2;
+ return src[9].fixed_hw_reg.dw1.ud;
else
return 1;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index 97df784..7cf3905 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -214,7 +214,7 @@ public:
fs_reg shadow_c,
fs_reg lod, fs_reg dpdy, int grad_components,
fs_reg sample_index,
- fs_reg offset,
+ fs_reg offset, int offset_components,
fs_reg mcs,
int gather_component,
bool is_cube_array,
diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index efa55b6..f5a4992 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1759,7 +1759,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
instr->is_array;
int lod_components = 0;
- int UNUSED offset_components = 0;
+ int offset_components = 0;
fs_reg coordinate, shadow_comparitor, lod, lod2, sample_index, mcs, tex_offset;
@@ -1808,10 +1808,6 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
break;
case nir_tex_src_offset:
tex_offset = retype(src, BRW_REGISTER_TYPE_D);
- if (instr->is_array)
- offset_components = instr->coord_components - 1;
- else
- offset_components = instr->coord_components;
break;
case nir_tex_src_projector:
unreachable("should be lowered");
@@ -1855,6 +1851,13 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
}
}
+ if (tex_offset.file != BAD_FILE) {
+ if (instr->is_array)
+ offset_components = instr->coord_components - 1;
+ else
+ offset_components = instr->coord_components;
+ }
+
enum glsl_base_type dest_base_type;
switch (instr->dest_type) {
case nir_type_float:
@@ -1892,7 +1895,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
emit_texture(op, dest_type, coordinate, instr->coord_components,
shadow_comparitor, lod, lod2, lod_components, sample_index,
- tex_offset, mcs, gather_component,
+ tex_offset, offset_components, mcs, gather_component,
is_cube_array, is_rect, sampler, sampler_reg, texunit);
fs_reg dest = get_nir_dest(instr->dest);
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 111db8c..ecd5239 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -215,7 +215,7 @@ fs_visitor::emit_texture(ir_texture_opcode op,
fs_reg shadow_c,
fs_reg lod, fs_reg lod2, int grad_components,
fs_reg sample_index,
- fs_reg offset_value,
+ fs_reg offset_value, int offset_components,
fs_reg mcs,
int gather_component,
bool is_cube_array,
@@ -259,6 +259,12 @@ fs_visitor::emit_texture(ir_texture_opcode op,
sampler, texunit);
}
+ /* The number of components of the texture offset should be the same as the
+ * number of derivative components where they are both present.
+ */
+ assert(!offset_components || !grad_components ||
+ offset_components == grad_components);
+
/* Writemasking doesn't eliminate channels on SIMD8 texture
* samples, so don't worry about them.
*/
@@ -266,7 +272,7 @@ fs_visitor::emit_texture(ir_texture_opcode op,
const fs_reg srcs[] = {
coordinate, shadow_c, lod, lod2,
sample_index, mcs, sampler_reg, offset_value,
- fs_reg(coord_components), fs_reg(grad_components)
+ fs_reg(coord_components), fs_reg(MAX2(offset_components, grad_components))
};
enum opcode opcode;
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
