On 28/07/15 23:49, Jason Ekstrand wrote:
> On Mon, Jul 27, 2015 at 3:37 PM, Alejandro Piñeiro <apinhe...@igalia.com>
> wrote:
>> Uses the nir structure to get all the info needed (sources,
>> dest reg, etc), and then it uses the common
>> vec4_visitor::emit_texture to emit the final code.
>> ---
>>
>> v3: added glsl_type_for_nir_alu_type helper as suggested (it could be reused
>> on fs too in the future). Setting with more detail the number of components
>> when calling get_nir_src. In the case of nir_tex_src_coord, nir_tex_src_ddx
>> and nir_tex_src_ddy, nir_tex_instr_src_size is used. I could have used
>> nir_tex_instr_src_size for all the sources, but I think that it would
>> be better to not call it if the number of components is always the same
>> for a given source type. Additionally, thanks to call get_nir_src with the
>> proper number of components, some manually set swizzles are gone.
>>
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 198
>> ++++++++++++++++++++++++++++-
>> 1 file changed, 197 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 16467ce5..b79a41e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -1281,10 +1281,206 @@ vec4_visitor::nir_emit_jump(nir_jump_instr *instr)
>> }
>> }
>>
>> +enum ir_texture_opcode
>> +ir_texture_opcode_for_nir_texop(nir_texop texop)
>> +{
>> + enum ir_texture_opcode op;
>> +
>> + switch (texop) {
>> + case nir_texop_lod: op = ir_lod; break;
>> + case nir_texop_query_levels: op = ir_query_levels; break;
>> + case nir_texop_tex: op = ir_tex; break;
>> + case nir_texop_tg4: op = ir_tg4; break;
>> + case nir_texop_txb: op = ir_txb; break;
>> + case nir_texop_txd: op = ir_txd; break;
>> + case nir_texop_txf: op = ir_txf; break;
>> + case nir_texop_txf_ms: op = ir_txf_ms; break;
>> + case nir_texop_txl: op = ir_txl; break;
>> + case nir_texop_txs: op = ir_txs; break;
>> + default:
>> + unreachable("unknown texture opcode");
>> + }
>> +
>> + return op;
>> +}
>> +const glsl_type *
>> +glsl_type_for_nir_alu_type(nir_alu_type alu_type,
>> + unsigned components)
>> +{
>> + switch (alu_type) {
>> + case nir_type_float:
>> + return glsl_type::vec(components);
>> + case nir_type_int:
>> + return glsl_type::ivec(components);
>> + case nir_type_unsigned:
>> + return glsl_type::uvec(components);
>> + case nir_type_bool:
>> + return glsl_type::bvec(components);
>> + default:
>> + return glsl_type::error_type;
>> + }
>> +
>> + return glsl_type::error_type;
>> +}
>> +
>> void
>> vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>> {
>> - /* @TODO: Not yet implemented */
>> + unsigned sampler = instr->sampler_index;
>> + src_reg sampler_reg = src_reg(sampler);
>> + src_reg coordinate;
>> + const glsl_type *coord_type = NULL;
>> + src_reg shadow_comparitor;
>> + src_reg offset_value;
>> + src_reg lod, lod2;
>> + src_reg sample_index;
>> + src_reg mcs;
>> +
>> + const glsl_type *dest_type =
>> + glsl_type_for_nir_alu_type(instr->dest_type,
>> + nir_tex_instr_dest_size(instr));
>> + dst_reg dest = get_nir_dest(instr->dest, instr->dest_type);
>> +
>> + /* When tg4 is used with the degenerate ZERO/ONE swizzles, don't bother
>> + * emitting anything other than setting up the constant result.
>> + */
>> + if (instr->op == nir_texop_tg4) {
>> + int swiz = GET_SWZ(key->tex.swizzles[sampler], instr->component);
>> + if (swiz == SWIZZLE_ZERO || swiz == SWIZZLE_ONE) {
>> + emit(MOV(dest, src_reg(swiz == SWIZZLE_ONE ? 1.0f : 0.0f)));
>> + return;
>> + }
>> + }
>> +
>> + /* Load the texture operation sources */
>> + for (unsigned i = 0; i < instr->num_srcs; i++) {
>> + switch (instr->src[i].src_type) {
>> + case nir_tex_src_comparitor:
>> + shadow_comparitor = get_nir_src(instr->src[i].src,
>> + BRW_REGISTER_TYPE_F, 1);
>> + break;
>> +
>> + case nir_tex_src_coord: {
>> + unsigned src_size = nir_tex_instr_src_size(instr, i);
>> +
>> + switch (instr->op) {
>> + case nir_texop_txf:
>> + case nir_texop_txf_ms:
>> + coordinate = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D,
>> + src_size);
>> + coord_type = glsl_type::ivec(src_size);
>> + break;
>> +
>> + default:
>> + coordinate = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_F,
>> + src_size);
>> + coord_type = glsl_type::vec(src_size);
>> + break;
>> + }
>> + break;
>> + }
>> +
>> + case nir_tex_src_ddx:
>> + lod = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_F,
>> + nir_tex_instr_src_size(instr, i));
>> + break;
>> +
>> + case nir_tex_src_ddy:
>> + lod2 = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_F,
>> + nir_tex_instr_src_size(instr, i));
>> + break;
>> +
>> + case nir_tex_src_lod:
>> + switch (instr->op) {
>> + case nir_texop_txs:
>> + lod = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_UD, 1);
>> + break;
>> +
>> + case nir_texop_txf:
>> + lod = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D, 1);
> Why is one of these signed and one unsigned?
In this aspect I used as reference brw_fs_nir:
http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/brw_fs_nir.cpp#n1639
that uses one as signed and other as unsigned. In any case
> In GLSL, both texelFetch
> and textureSize take a signed LOD. Can we combine the two cases?
Yes, makes sense. And I made a piglit run just in case, without regressions.
>> + break;
>> +
>> + default:
>> + lod = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_F, 1);
>> + break;
>> + }
>> + break;
>> +
>> + case nir_tex_src_ms_index: {
>> + sample_index = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D,
>> 1);
>> + assert(coord_type != NULL);
>> + if (devinfo->gen >= 7 &&
>> + key->tex.compressed_multisample_layout_mask & (1<<sampler)) {
>> + mcs = emit_mcs_fetch(coord_type, coordinate, sampler_reg);
>> + } else {
>> + mcs = src_reg(0u);
>> + }
>> + mcs = retype(mcs, BRW_REGISTER_TYPE_UD);
>> + break;
>> + }
>> +
>> + case nir_tex_src_offset:
>> + offset_value = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D,
>> 2);
>> + break;
>> +
>> + case nir_tex_src_sampler_offset: {
>> + /* The highest sampler which may be used by this operation is
>> + * the last element of the array. Mark it here, because the
>> generator
>> + * doesn't have enough information to determine the bound.
>> + */
>> + uint32_t array_size = instr->sampler_array_size;
>> + src_reg src = get_nir_src(instr->src[i].src, 1);
> Let's move this down a few lines, closer to where it's used.
Ok. Done.
>
> Other than the two comments above,
>
> Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com>
Thanks for the quick review.
>
>> + uint32_t max_used = sampler + array_size - 1;
>> + if (instr->op == nir_texop_tg4) {
>> + max_used += prog_data->base.binding_table.gather_texture_start;
>> + } else {
>> + max_used += prog_data->base.binding_table.texture_start;
>> + }
>> +
>> + brw_mark_surface_used(&prog_data->base, max_used);
>> +
>> + /* Emit code to evaluate the actual indexing expression */
>> + src_reg temp(this, glsl_type::uint_type);
>> + emit(ADD(dst_reg(temp), src, src_reg(sampler)));
>> + sampler_reg = emit_uniformize(temp);
>> + break;
>> + }
>> +
>> + case nir_tex_src_projector:
>> + unreachable("Should be lowered by do_lower_texture_projection");
>> +
>> + case nir_tex_src_bias:
>> + unreachable("LOD bias is not valid for vertex shaders.\n");
>> +
>> + default:
>> + unreachable("unknown texture source");
>> + }
>> + }
>> +
>> + uint32_t constant_offset = 0;
>> + for (unsigned i = 0; i < 3; i++) {
>> + if (instr->const_offset[i] != 0) {
>> + constant_offset = brw_texture_offset(instr->const_offset, 3);
>> + break;
>> + }
>> + }
>> +
>> + /* Stuff the channel select bits in the top of the texture offset */
>> + if (instr->op == nir_texop_tg4)
>> + constant_offset |= gather_channel(instr->component, sampler) << 16;
>> +
>> + ir_texture_opcode op = ir_texture_opcode_for_nir_texop(instr->op);
>> +
>> + bool is_cube_array =
>> + instr->op == nir_texop_txs &&
>> + instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE &&
>> + instr->is_array;
>> +
>> + emit_texture(op, dest, dest_type, coordinate, instr->coord_components,
>> + shadow_comparitor,
>> + lod, lod2, sample_index,
>> + constant_offset, offset_value,
>> + mcs, is_cube_array, sampler, sampler_reg);
>> }
>>
>> }
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Alejandro Piñeiro (apinhe...@igalia.com)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
