[Mesa-dev] [PATCH 61/78] i965/nir/vec4: Add skeleton implementation of nir_emit_texture()
Jason Ekstrand
jason at jlekstrand.net
Mon Jul 13 07:17:49 PDT 2015
On Jul 13, 2015 7:14 AM, "Alejandro Piñeiro" <apinheiro at igalia.com> wrote:
>
>
>
> On 13/07/15 14:05, Jason Ekstrand wrote:
> > On Mon, Jul 13, 2015 at 3:00 AM, Alejandro Piñeiro <apinheiro at igalia.com>
wrote:
> >> On 01/07/15 01:37, Jason Ekstrand wrote:
> >>> If we can avoid duplication in the texturing code, that would be
> >>> really nice. Could we do this as a refactor of the old code and then
> >>> a much smaller NIR function that calls some shared code? That's what
> >>> we did for FS and it worked ok. I looked at the layout of your code
> >>> and, after you finish reading instruction inputs, very little of it is
> >>> actually NIR-specific.
> >> Our wip second batch of commits has now a version that refactors the
old
> >> code. But I have a doubt. As part of this refactor, I created a new
> >> method to be used by brw_fs and brw_vec4, that returns the ir texture
> >> opcode based on the nir texture opcode. You can take a look to that
> >> method here [1]. The issue with this method is that ir_texture_opcode
is
> >> defined at glsl/ir.h inside a #ifdef __cplusplus. So I needed to rename
> >> brw_nir.c to brw_nir.cpp. On a previous early review [2] you mentioned
> >> that it would be better to keep it as brw_nir.c unless we have a good
> >> reason. Is using this enum good reason to do the renaming? If not, the
> >> other option I see is modify glsl/ir.h and move the enum definitions to
> >> the top, outside the ifdef __cplusplus, as other headers (like
> >> glsl/glsl_types.h) have.
> > Go ahead and make it a cpp file if you have to.
>
> Ok, thanks.
>
> > Another option would
> > be to do the map the other way: ir_opcode -> nir_tex_op and refactor
> > the code to use the nir_tex_op.
>
> But even if we do the map in the other way, the helper function would be
> using the enum type defined at glsl/ir.h, so we would need to make it a
> cpp file in any case. So for now, I will keep things simple, and keep
> the current mapping, and do the change later.
The difference is that, with the patch I sent a few hours ago, you only
need to do the conversion one place so there's no need for the helper.
> > That's probably what we should be
> > doing in the FS now that the old visitor is gone.
> > --Jason
> >
> >> BR
> >>
> >> [1]
> >>
https://github.com/Igalia/mesa/commit/e81ce150ef931e50b6cb1aae0b42a79f448863f5
> >> [2] https://bugs.freedesktop.org/show_bug.cgi?id=89580#c7
> >>> --Jason
> >>>
> >>> On Fri, Jun 26, 2015 at 1:07 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:
> >>>> From: Alejandro Piñeiro <apinheiro at igalia.com>
> >>>>
> >>>> This sets up the basic structure and placeholders for the
implementation of
> >>>> texture operations, which will be added incrementally in subsequent
patches.
> >>>>
> >>>> Apart from helping split the texture support into smaller patches,
this patch
> >>>> is useful to get a simplified picture of the steps involved in the
emission of
> >>>> texture operations.
> >>>>
> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> >>>> ---
> >>>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 120
++++++++++++++++++++++++++++-
> >>>> 1 file changed, 119 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 08a70b0..c3638a0 100644
> >>>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >>>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >>>> @@ -1412,7 +1412,125 @@
vec4_visitor::nir_swizzle_result(nir_tex_instr *instr, dst_reg dest,
> >>>> 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);
> >>>> +
> >>>> + const glsl_type *dest_type;
> >>>> +
> >>>> + /* Load the texture operation sources */
> >>>> + for (unsigned i = 0; i < instr->num_srcs; i++) {
> >>>> + src_reg src = get_nir_src(instr->src[i].src);
> >>>> +
> >>>> + switch (instr->src[i].src_type) {
> >>>> + case nir_tex_src_comparitor:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_coord:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_ddx:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_ddy:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_lod:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_ms_index:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_offset:
> >>>> + /* @TODO: not yet implemented */
> >>>> + break;
> >>>> +
> >>>> + case nir_tex_src_sampler_offset:
> >>>> + /* @TODO: not yet implemented */
> >>>> + 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");
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + /* Get the texture operation opcode */
> >>>> + enum opcode opcode = shader_opcode_for_nir_opcode(instr->op);
> >>>> +
> >>>> + /* Build the instruction */
> >>>> + enum glsl_base_type dest_base_type =
> >>>> + brw_glsl_base_type_for_nir_type(instr->dest_type);
> >>>> +
> >>>> + dest_type =
> >>>> + glsl_type::get_instance(dest_base_type,
nir_tex_instr_dest_size(instr), 1);
> >>>> +
> >>>> + vec4_instruction *inst = new(mem_ctx)
> >>>> + vec4_instruction(opcode, dst_reg(this, dest_type));
> >>>> +
> >>>> + for (unsigned i = 0; i < 3; i++) {
> >>>> + if (instr->const_offset[i] != 0) {
> >>>> + inst->offset = brw_texture_offset(instr->const_offset, 3);
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + /* The message header is necessary for:
> >>>> + * - Texel offsets
> >>>> + * - Gather channel selection
> >>>> + * - Sampler indices too large to fit in a 4-bit value.
> >>>> + */
> >>>> + inst->header_size =
> >>>> + (inst->offset != 0 ||
> >>>> + instr->op == nir_texop_tg4 ||
> >>>> + is_high_sampler(sampler_reg)) ? 1 : 0;
> >>>> + inst->base_mrf = 2;
> >>>> + inst->mlen = inst->header_size + 1;
> >>>> + inst->dst.writemask = WRITEMASK_XYZW;
> >>>> +
> >>>> + inst->src[1] = sampler_reg;
> >>>> +
> >>>> + /* Calculate the MRF for the first parameter */
> >>>> + int param_base = inst->base_mrf + inst->header_size;
> >>>> +
> >>>> + if (instr->op == nir_texop_txs || instr->op ==
nir_texop_query_levels) {
> >>>> + /* @TODO: not yet implemented */
> >>>> + } else {
> >>>> + /* @TODO: Load the coordinate */
> >>>> +
> >>>> + /* @TODO: Load the shadow comparitor */
> >>>> +
> >>>> + /* Load the LOD info */
> >>>> + if (instr->op == nir_texop_tex || instr->op == nir_texop_txl)
{
> >>>> + /* @TODO: not yet implemented */
> >>>> + } else if (instr->op == nir_texop_txf) {
> >>>> + /* @TODO: not yet implemented */
> >>>> + } else if (instr->op == nir_texop_txf_ms) {
> >>>> + /* @TODO: not yet implemented */
> >>>> + } else if (instr->op == nir_texop_txd) {
> >>>> + /* @TODO: not yet implemented */
> >>>> + } else if (instr->op == nir_texop_tg4) {
> >>>> + /* @TODO: not yet implemented */
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + /* Emit the instruction */
> >>>> + emit(inst);
> >>>> +
> >>>> + /* Move the result to the destination registry, applying swizzle
*/
> >>>> + dst_reg dest = get_nir_dest(instr->dest,
brw_type_for_base_type(dest_type));
> >>>> +
> >>>> + nir_swizzle_result(instr, dest, src_reg(inst->dst), sampler,
dest_type);
> >>>> }
> >>>>
> >>>> }
> >>>> --
> >>>> 2.1.4
> >>>>
> >>>> _______________________________________________
> >>>> mesa-dev mailing list
> >>>> mesa-dev at lists.freedesktop.org
> >>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>> _______________________________________________
> >>> mesa-dev mailing list
> >>> mesa-dev at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >> --
> >> Alejandro Piñeiro (apinheiro at igalia.com)
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> --
> Alejandro Piñeiro (apinheiro at igalia.com)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150713/db947c64/attachment-0001.html>
More information about the mesa-dev
mailing list