Francisco Jerez <[email protected]> writes: > Kenneth Graunke <[email protected]> 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
