[Mesa-dev] [PATCH 18/20] i965/fs: Translate image load, store and atomic NIR intrinsics.
Francisco Jerez
currojerez at riseup.net
Thu Jul 23 05:40:26 PDT 2015
Jason Ekstrand <jason at jlekstrand.net> writes:
> On Tue, Jul 21, 2015 at 9:38 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> ---
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 149 +++++++++++++++++++++++++++++++
>> 1 file changed, 149 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 31024b7..76297b7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -24,6 +24,7 @@
>> #include "glsl/ir.h"
>> #include "glsl/ir_optimization.h"
>> #include "glsl/nir/glsl_to_nir.h"
>> +#include "main/shaderimage.h"
>> #include "program/prog_to_nir.h"
>> #include "brw_fs.h"
>> #include "brw_fs_surface_builder.h"
>> @@ -1245,6 +1246,97 @@ fs_visitor::emit_percomp(const fs_builder &bld, const fs_inst &inst,
>> }
>> }
>>
>> +/**
>> + * Get the matching channel register datatype for an image intrinsic of the
>> + * specified GLSL image type.
>> + */
>> +static brw_reg_type
>> +get_image_base_type(const glsl_type *type)
>> +{
>> + switch ((glsl_base_type)type->sampler_type) {
>> + case GLSL_TYPE_UINT:
>> + return BRW_REGISTER_TYPE_UD;
>> + case GLSL_TYPE_INT:
>> + return BRW_REGISTER_TYPE_D;
>> + case GLSL_TYPE_FLOAT:
>> + return BRW_REGISTER_TYPE_F;
>> + default:
>> + unreachable("Not reached.");
>> + }
>> +}
>> +
>> +/**
>> + * Get the appropriate atomic op for an image atomic intrinsic.
>> + */
>> +static unsigned
>> +get_image_atomic_op(nir_intrinsic_op op, const glsl_type *type)
>> +{
>> + switch (op) {
>> + case nir_intrinsic_image_atomic_add:
>> + return BRW_AOP_ADD;
>> + case nir_intrinsic_image_atomic_min:
>> + return (get_image_base_type(type) == BRW_REGISTER_TYPE_D ?
>> + BRW_AOP_IMIN : BRW_AOP_UMIN);
>> + case nir_intrinsic_image_atomic_max:
>> + return (get_image_base_type(type) == BRW_REGISTER_TYPE_D ?
>> + BRW_AOP_IMAX : BRW_AOP_UMAX);
>> + case nir_intrinsic_image_atomic_and:
>> + return BRW_AOP_AND;
>> + case nir_intrinsic_image_atomic_or:
>> + return BRW_AOP_OR;
>> + case nir_intrinsic_image_atomic_xor:
>> + return BRW_AOP_XOR;
>> + case nir_intrinsic_image_atomic_exchange:
>> + return BRW_AOP_MOV;
>> + case nir_intrinsic_image_atomic_comp_swap:
>> + return BRW_AOP_CMPWR;
>> + default:
>> + unreachable("Not reachable.");
>> + }
>> +}
>> +
>> +/**
>> + * Return true if the image is a 1D array and the implementation
>> + * requires the array index to be passed in as the Z component of the
>> + * coordinate vector.
>> + */
>> +static bool
>> +needs_zero_y_image_coordinate(const fs_builder &bld,
>> + const nir_variable *image)
>> +{
>> + const glsl_type *type = image->type->without_array();
>> + const mesa_format format =
>> + _mesa_get_shader_image_format(image->data.image.format);
>> + /* HSW in vec4 mode and our software coordinate handling for untyped
>> + * reads want the array index to be at the Z component.
>> + */
>> + const bool array_index_at_z = (!image->data.image.write_only &&
>> + !image_format_info::has_matching_typed_format(
>> + bld.shader->devinfo, format));
>> +
>> + return (type->sampler_dimensionality == GLSL_SAMPLER_DIM_1D &&
>> + type->sampler_array && array_index_at_z);
>> +}
>> +
>> +/**
>> + * Transform image coordinates into the form expected by the
>> + * implementation.
>> + */
>> +static fs_reg
>> +fix_image_address(const fs_builder &bld,
>> + const nir_variable *image, const fs_reg &addr)
>
> Is there a good reason why this is happening here and not inside of
> emit_image_load/store? It seems to break encaptulation substantially
> if the code that calls emit_image_load/store has to think about this
> stuff and adjust the address type based on whether a typed or untyped
> message will eventually be used.
>
Yeah, I'm not particularly fond of this either, but there's a reason:
For the surface builder library images are simply an N-dimensional array
of texels (with N being the dims argument of emit_image_load, store and
atomic), so they don't have a chance to apply this work-around because
they cannot tell whether the image is a plain N-dimensional texture or
an array of N-1-dimensional textures.
With something like array_reg it would be easy to factor out this
workaround into an (addr, dims) -> (addr, dims) mapping function and
punt it to brw_fs_surface_builder.cpp, but that doesn't seem to be an
option, so we're left with either doing it here or adding an explicit
array_dims argument to the image functions and apply this work-around in
each of them, which isn't going to be nice either.
>> +{
>> + if (needs_zero_y_image_coordinate(bld, image)) {
>> + assert(image->type->without_array()->coordinate_components() == 2);
>> + const fs_reg srcs[] = { addr, fs_reg(0), offset(addr, bld, 1) };
>> + const fs_reg tmp = bld.vgrf(addr.type, ARRAY_SIZE(srcs));
>> + bld.LOAD_PAYLOAD(tmp, srcs, ARRAY_SIZE(srcs), 0);
>> + return tmp;
>> + } else {
>> + return addr;
>> + }
>> +}
>> +
>> void
>> fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr)
>> {
>> @@ -1319,6 +1411,63 @@ fs_visitor::nir_emit_intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr
>> break;
>> }
>>
>> + case nir_intrinsic_image_load:
>> + case nir_intrinsic_image_store:
>> + case nir_intrinsic_image_atomic_add:
>> + case nir_intrinsic_image_atomic_min:
>> + case nir_intrinsic_image_atomic_max:
>> + case nir_intrinsic_image_atomic_and:
>> + case nir_intrinsic_image_atomic_or:
>> + case nir_intrinsic_image_atomic_xor:
>> + case nir_intrinsic_image_atomic_exchange:
>> + case nir_intrinsic_image_atomic_comp_swap: {
>> + using namespace image_access;
>> +
>> + /* Get the referenced image variable and type. */
>> + const nir_variable *var = instr->variables[0]->var;
>> + const glsl_type *type = var->type->without_array();
>> + const brw_reg_type base_type = get_image_base_type(type);
>> +
>> + /* Get some metadata from the image intrinsic. */
>> + const nir_intrinsic_info *info = &nir_intrinsic_infos[instr->intrinsic];
>> + const unsigned dims = (type->coordinate_components() +
>> + (needs_zero_y_image_coordinate(bld, var) ? 1 : 0));
>> + const mesa_format format =
>> + (var->data.image.write_only ? MESA_FORMAT_NONE :
>> + _mesa_get_shader_image_format(var->data.image.format));
>> +
>> + /* Get the arguments of the image intrinsic. */
>> + const fs_reg image = get_nir_image_deref(instr->variables[0]);
>> + const fs_reg addr = fix_image_address(bld, var,
>> + retype(get_nir_src(instr->src[0]),
>> + BRW_REGISTER_TYPE_UD));
>> + const fs_reg src0 = (info->num_srcs >= 3 ?
>> + retype(get_nir_src(instr->src[2]), base_type) :
>> + fs_reg());
>> + const fs_reg src1 = (info->num_srcs >= 4 ?
>> + retype(get_nir_src(instr->src[3]), base_type) :
>> + fs_reg());
>> + fs_reg tmp;
>> +
>> + /* Emit an image load, store or atomic op. */
>> + if (instr->intrinsic == nir_intrinsic_image_load)
>> + tmp = emit_image_load(bld, image, addr, format, dims);
>> +
>> + else if (instr->intrinsic == nir_intrinsic_image_store)
>> + emit_image_store(bld, image, addr, src0, format, dims);
>> +
>> + else
>> + tmp = emit_image_atomic(bld, image, addr, src0, src1,
>> + dims, info->dest_components,
>> + get_image_atomic_op(instr->intrinsic, type));
>> +
>> + /* Assign the result. */
>> + for (unsigned c = 0; c < info->dest_components; ++c)
>> + bld.MOV(offset(retype(dest, base_type), bld, c),
>> + offset(tmp, bld, c));
>> + break;
>> + }
>> +
>> case nir_intrinsic_load_front_face:
>> bld.MOV(retype(dest, BRW_REGISTER_TYPE_D),
>> *emit_frontfacing_interpolation());
>> --
>> 2.4.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/20150723/0b1dd2fc/attachment-0001.sig>
More information about the mesa-dev
mailing list