[Mesa-dev] [PATCH 004/133] nir: add the core datastructures
Jason Ekstrand
jason at jlekstrand.net
Fri Dec 19 17:04:50 PST 2014
Glenn,
Thanks for taking the time to look over things.
On Fri, Dec 19, 2014 at 1:24 PM, Glenn Kennard <glenn.kennard at gmail.com>
wrote:
>
> On Tue, 16 Dec 2014 07:04:14 +0100, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
>
> From: Connor Abbott <connor.abbott at intel.com>
>>
>> This includes all the instructions, ifs, loops, functions, etc. This is
>> similar to the information in ir.h.
>>
>> v2: Jason Ekstrand <jason.ekstrand at intel.com>:
>> Include ralloc and hash_table from the util directory
>> ---
>> src/glsl/Makefile.sources | 2 +
>> src/glsl/nir/nir.h | 1150 ++++++++++++++++++++++++++++++
>> +++++++++++
>> src/glsl/nir/nir_intrinsics.c | 49 ++
>> src/glsl/nir/nir_intrinsics.h | 158 ++++++
>> src/glsl/nir/nir_opcodes.c | 46 ++
>> src/glsl/nir/nir_opcodes.h | 346 +++++++++++++
>> 6 files changed, 1751 insertions(+)
>> create mode 100644 src/glsl/nir/nir.h
>> create mode 100644 src/glsl/nir/nir_intrinsics.c
>> create mode 100644 src/glsl/nir/nir_intrinsics.h
>> create mode 100644 src/glsl/nir/nir_opcodes.c
>> create mode 100644 src/glsl/nir/nir_opcodes.h
>>
>> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
>> index c3a90f7..e8eedd1 100644
>> --- a/src/glsl/Makefile.sources
>> +++ b/src/glsl/Makefile.sources
>> @@ -14,6 +14,8 @@ LIBGLCPP_GENERATED_FILES = \
>> $(GLSL_BUILDDIR)/glcpp/glcpp-parse.c
>> NIR_FILES = \
>> + $(GLSL_SRCDIR)/nir/nir_intrinsics.c \
>> + $(GLSL_SRCDIR)/nir/nir_opcodes.c \
>> $(GLSL_SRCDIR)/nir/nir_types.cpp
>> # libglsl
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> new file mode 100644
>> index 0000000..ef486da
>> --- /dev/null
>> +++ b/src/glsl/nir/nir.h
>> @@ -0,0 +1,1150 @@
>> +/*
>> + * Copyright © 2014 Connor Abbott
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> next
>> + * paragraph) shall be included in all copies or substantial portions of
>> the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Connor Abbott (cwabbott0 at gmail.com)
>> + *
>> + */
>> +
>> +#pragma once
>> +
>> +#include "util/hash_table.h"
>> +#include "main/set.h"
>> +#include "../list.h"
>> +#include "GL/gl.h" /* GLenum */
>> +#include "util/ralloc.h"
>> +#include "nir_types.h"
>> +#include <stdio.h>
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +struct nir_function_overload;
>> +struct nir_function;
>> +
>> +
>> +/**
>> + * Description of built-in state associated with a uniform
>> + *
>> + * \sa nir_variable::state_slots
>> + */
>> +typedef struct {
>> + int tokens[5];
>> + int swizzle;
>> +} nir_state_slot;
>> +
>> +typedef enum {
>> + nir_var_shader_in,
>> + nir_var_shader_out,
>> + nir_var_global,
>> + nir_var_local,
>> + nir_var_uniform,
>> + nir_var_system_value
>> +} nir_variable_mode;
>> +
>> +/**
>> + * Data stored in an nir_constant
>> + */
>> +union nir_constant_data {
>> + unsigned u[16];
>> + int i[16];
>> + float f[16];
>> + bool b[16];
>> +};
>> +
>> +typedef struct nir_constant {
>> + /**
>> + * Value of the constant.
>> + *
>> + * The field used to back the values supplied by the constant is
>> determined
>> + * by the type associated with the \c ir_instruction. Constants may
>> be
>> + * scalars, vectors, or matrices.
>> + */
>> + union nir_constant_data value;
>> +
>> + /* Array elements / Structure Fields */
>> + struct nir_constant **elements;
>> +} nir_constant;
>> +
>> +/**
>> + * \brief Layout qualifiers for gl_FragDepth.
>> + *
>> + * The AMD/ARB_conservative_depth extensions allow gl_FragDepth to be
>> redeclared
>> + * with a layout qualifier.
>> + */
>> +typedef enum {
>> + nir_depth_layout_none, /**< No depth layout is specified. */
>> + nir_depth_layout_any,
>> + nir_depth_layout_greater,
>> + nir_depth_layout_less,
>> + nir_depth_layout_unchanged
>> +} nir_depth_layout;
>> +
>> +/**
>> + * Either a uniform, global variable, shader input, or shader output.
>> Based on
>> + * ir_variable - it should be easy to translate between the two.
>> + */
>> +
>> +typedef struct {
>> + struct exec_node node;
>> +
>> + /**
>> + * Declared type of the variable
>> + */
>> + const struct glsl_type *type;
>> +
>> + /**
>> + * Declared name of the variable
>> + */
>> + char *name;
>> +
>> + /**
>> + * For variables which satisfy the is_interface_instance() predicate,
>> this
>> + * points to an array of integers such that if the ith member of the
>> + * interface block is an array, max_ifc_array_access[i] is the maximum
>> + * array element of that member that has been accessed. If the ith
>> member
>> + * of the interface block is not an array, max_ifc_array_access[i] is
>> + * unused.
>> + *
>> + * For variables whose type is not an interface block, this pointer is
>> + * NULL.
>> + */
>> + unsigned *max_ifc_array_access;
>> +
>> + struct nir_variable_data {
>> +
>> + /**
>> + * Is the variable read-only?
>> + *
>> + * This is set for variables declared as \c const, shader inputs,
>> + * and uniforms.
>> + */
>> + unsigned read_only:1;
>> + unsigned centroid:1;
>> + unsigned sample:1;
>> + unsigned invariant:1;
>> +
>> + /**
>> + * Storage class of the variable.
>> + *
>> + * \sa nir_variable_mode
>> + */
>> + unsigned mode:4;
>> +
>> + /**
>> + * Interpolation mode for shader inputs / outputs
>> + *
>> + * \sa ir_variable_interpolation
>> + */
>> + unsigned interpolation:2;
>> +
>> + /**
>> + * \name ARB_fragment_coord_conventions
>> + * @{
>> + */
>> + unsigned origin_upper_left:1;
>> + unsigned pixel_center_integer:1;
>> + /*@}*/
>> +
>> + /**
>> + * Was the location explicitly set in the shader?
>> + *
>> + * If the location is explicitly set in the shader, it \b cannot
>> be changed
>> + * by the linker or by the API (e.g., calls to \c
>> glBindAttribLocation have
>> + * no effect).
>> + */
>> + unsigned explicit_location:1;
>> + unsigned explicit_index:1;
>> +
>> + /**
>> + * Was an initial binding explicitly set in the shader?
>> + *
>> + * If so, constant_value contains an integer ir_constant
>> representing the
>> + * initial binding point.
>> + */
>> + unsigned explicit_binding:1;
>> +
>> + /**
>> + * Does this variable have an initializer?
>> + *
>> + * This is used by the linker to cross-validiate initializers of
>> global
>> + * variables.
>> + */
>> + unsigned has_initializer:1;
>> +
>> + /**
>> + * Is this variable a generic output or input that has not yet
>> been matched
>> + * up to a variable in another stage of the pipeline?
>> + *
>> + * This is used by the linker as scratch storage while assigning
>> locations
>> + * to generic inputs and outputs.
>> + */
>> + unsigned is_unmatched_generic_inout:1;
>> +
>> + /**
>> + * If non-zero, then this variable may be packed along with other
>> variables
>> + * into a single varying slot, so this offset should be applied
>> when
>> + * accessing components. For example, an offset of 1 means that
>> the x
>> + * component of this variable is actually stored in component y of
>> the
>> + * location specified by \c location.
>> + */
>> + unsigned location_frac:2;
>> +
>> + /**
>> + * Non-zero if this variable was created by lowering a named
>> interface
>> + * block which was not an array.
>> + *
>> + * Note that this variable and \c from_named_ifc_block_array will
>> never
>> + * both be non-zero.
>> + */
>> + unsigned from_named_ifc_block_nonarray:1;
>> +
>> + /**
>> + * Non-zero if this variable was created by lowering a named
>> interface
>> + * block which was an array.
>> + *
>> + * Note that this variable and \c from_named_ifc_block_nonarray
>> will never
>> + * both be non-zero.
>> + */
>> + unsigned from_named_ifc_block_array:1;
>> +
>> + /**
>> + * \brief Layout qualifier for gl_FragDepth.
>> + *
>> + * This is not equal to \c ir_depth_layout_none if and only if this
>> + * variable is \c gl_FragDepth and a layout qualifier is specified.
>> + */
>> + nir_depth_layout depth_layout;
>> +
>> + /**
>> + * Storage location of the base of this variable
>> + *
>> + * The precise meaning of this field depends on the nature of the
>> variable.
>> + *
>> + * - Vertex shader input: one of the values from \c
>> gl_vert_attrib.
>> + * - Vertex shader output: one of the values from \c
>> gl_varying_slot.
>> + * - Geometry shader input: one of the values from \c
>> gl_varying_slot.
>> + * - Geometry shader output: one of the values from \c
>> gl_varying_slot.
>> + * - Fragment shader input: one of the values from \c
>> gl_varying_slot.
>> + * - Fragment shader output: one of the values from \c
>> gl_frag_result.
>> + * - Uniforms: Per-stage uniform slot number for default uniform
>> block.
>> + * - Uniforms: Index within the uniform block definition for UBO
>> members.
>> + * - Other: This field is not currently used.
>> + *
>> + * If the variable is a uniform, shader input, or shader output,
>> and the
>> + * slot has not been assigned, the value will be -1.
>> + */
>> + int location;
>> +
>> + /**
>> + * The actual location of the variable in the IR. Only valid for
>> inputs
>> + * and outputs.
>> + */
>> + unsigned int driver_location;
>> +
>> + /**
>> + * output index for dual source blending.
>> + */
>> + int index;
>>
>
> src index is only ever 0 or 1. I'll note its already a bitfield in
> src/glsl/ir.h from where this is copied, along with some other space
> optimizations, might want to copy those over as well.
>
> +
>> + /**
>> + * Initial binding point for a sampler or UBO.
>> + *
>> + * For array types, this represents the binding point for the
>> first element.
>> + */
>> + int binding;
>> +
>> + /**
>> + * Location an atomic counter is stored at.
>> + */
>> + struct {
>> + unsigned buffer_index;
>> + unsigned offset;
>> + } atomic;
>> +
>> + /**
>> + * ARB_shader_image_load_store qualifiers.
>> + */
>> + struct {
>> + bool read_only; /**< "readonly" qualifier. */
>> + bool write_only; /**< "writeonly" qualifier. */
>> + bool coherent;
>> + bool _volatile;
>> + bool restrict_flag;
>> +
>> + /** Image internal format if specified explicitly, otherwise
>> GL_NONE. */
>> + GLenum format;
>> + } image;
>>
>
> Similar to dual source index, image struct has diverged a bit from
> src/glsl/ir.h.
>
> +
>> + /**
>> + * Highest element accessed with a constant expression array index
>> + *
>> + * Not used for non-array variables.
>> + */
>> + unsigned max_array_access;
>> +
>> + } data;
>> +
>> + /**
>> + * Built-in state that backs this uniform
>> + *
>> + * Once set at variable creation, \c state_slots must remain
>> invariant.
>> + * This is because, ideally, this array would be shared by all clones
>> of
>> + * this variable in the IR tree. In other words, we'd really like
>> for it
>> + * to be a fly-weight.
>> + *
>> + * If the variable is not a uniform, \c num_state_slots will be zero
>> and
>> + * \c state_slots will be \c NULL.
>> + */
>> + /*@{*/
>> + unsigned num_state_slots; /**< Number of state slots used */
>> + nir_state_slot *state_slots; /**< State descriptors. */
>> + /*@}*/
>> +
>> + /**
>> + * Value assigned in the initializer of a variable declared "const"
>> + */
>> + nir_constant *constant_value;
>> +
>> + /**
>> + * Constant expression assigned in the initializer of the variable
>> + *
>> + * \warning
>> + * This field and \c ::constant_value are distinct. Even if the two
>> fields
>> + * refer to constants with the same value, they must point to separate
>> + * objects.
>> + */
>> + nir_constant *constant_initializer;
>> +
>> + /**
>> + * For variables that are in an interface block or are an instance of
>> an
>> + * interface block, this is the \c GLSL_TYPE_INTERFACE type for that
>> block.
>> + *
>> + * \sa ir_variable::location
>> + */
>> + const struct glsl_type *interface_type;
>> +} nir_variable;
>> +
>> +typedef struct {
>> + struct exec_node node;
>> +
>> + unsigned num_components; /** < number of vector components */
>> + unsigned num_array_elems; /** < size of array (0 for no array) */
>> +
>> + /** for liveness analysis, the index in the bit-array of live
>> variables */
>> + unsigned index;
>> +
>> + /** only for debug purposes, can be NULL */
>> + const char *name;
>> +
>> + /** whether this register is local (per-function) or global
>> (per-shader) */
>> + bool is_global;
>> +
>> + /**
>> + * If this flag is set to true, then accessing channels >=
>> num_components
>> + * is well-defined, and simply spills over to the next array element.
>> This
>> + * is useful for backends that can do per-component accessing, in
>> + * particular scalar backends. By setting this flag and making
>> + * num_components equal to 1, structures can be packed tightly into
>> + * registers and then registers can be accessed per-component to get
>> to
>> + * each structure member, even if it crosses vec4 boundaries.
>> + */
>> + bool is_packed;
>> +
>> + /** set of nir_instr's where this register is used (read from) */
>> + struct set *uses;
>> +
>> + /** set of nir_instr's where this register is defined (written to) */
>> + struct set *defs;
>> +
>> + /** set of ifs where this register is used as a condition */
>> + struct set *if_uses;
>> +} nir_register;
>> +
>> +typedef enum {
>> + nir_instr_type_alu,
>> + nir_instr_type_call,
>> + nir_instr_type_texture,
>> + nir_instr_type_intrinsic,
>> + nir_instr_type_load_const,
>> + nir_instr_type_jump,
>> + nir_instr_type_ssa_undef,
>> + nir_instr_type_phi,
>> +} nir_instr_type;
>> +
>> +typedef struct {
>> + struct exec_node node;
>> + nir_instr_type type;
>> + struct nir_block *block;
>> +} nir_instr;
>> +
>> +#define nir_instr_next(instr) \
>> + exec_node_data(nir_instr, (instr)->node.next, node)
>> +
>> +#define nir_instr_prev(instr) \
>> + exec_node_data(nir_instr, (instr)->node.prev, node)
>>
>
> Static inlines similar what is done for nir_deref_as_var might make
> walking the structures in gdb a little easier.
>
> +
>> +typedef struct {
>> + /** for debugging only, can be NULL */
>> + const char* name;
>> +
>> + /** index into the bit-array for liveness analysis */
>> + unsigned index;
>> +
>> + nir_instr *parent_instr;
>> +
>> + struct set *uses;
>> + struct set *if_uses;
>>
>
> Comment what type the sets are, similar to whats already done for
> nir_register.uses.
>
Sure, that's a good plan.
> I think using sets for these is probably overkill, a simple array or
> linked list is likely better performing for most cases. A number or two to
> motivate the extra complexity perhaps?
>
Being able to quickly add/remove elements is extremely important. Just ask
the noveau people about this. Why not a linked list? Part of the problem
is that we have a many-to-many mapping. Multiple instructions can use the
same source and a single instruction can use multiple sources. I did think
about changing to a linked list with the link embedded in the source.
However, this would require adding 3 pointers to every source (prev, next,
and a pointer back to the instruction) in order be able to do it
efficiently. Also, if you want to iterate over all of the instructions
that use a source, you have to keep track of what instructions you've seen
because the same instruction can use a source multiple times. In other
words, It was deemed not practical.
>
> +
>> + uint8_t num_components;
>> +} nir_ssa_def;
>> +
>> +struct nir_src;
>> +
>> +typedef struct {
>> + nir_register *reg;
>> + struct nir_src *indirect; /** < NULL for no indirect offset */
>> + unsigned base_offset;
>> +
>> + /* TODO use-def chain goes here */
>>
>
> Stale comment? Same for nir_reg_dest
>
>
> +} nir_reg_src;
>> +
>> +typedef struct {
>> + nir_register *reg;
>> + struct nir_src *indirect; /** < NULL for no indirect offset */
>> + unsigned base_offset;
>> +
>> + /* TODO def-use chain goes here */
>> +} nir_reg_dest;
>> +
>> +typedef struct nir_src {
>> + union {
>> + nir_reg_src reg;
>> + nir_ssa_def *ssa;
>> + };
>> +
>> + bool is_ssa;
>> +} nir_src;
>> +
>> +typedef struct {
>> + union {
>> + nir_reg_dest reg;
>> + nir_ssa_def ssa;
>> + };
>> +
>> + bool is_ssa;
>> +} nir_dest;
>> +
>> +nir_src nir_src_copy(nir_src src, void *mem_ctx);
>> +nir_dest nir_dest_copy(nir_dest dest, void *mem_ctx);
>> +
>> +typedef struct {
>> + nir_src src;
>> +
>> + /**
>> + * \name input modifiers
>> + */
>> + /*@{*/
>> + /**
>> + * For inputs interpreted as a floating point, flips the sign bit.
>> For inputs
>> + * interpreted as an integer, performs the two's complement negation.
>> + */
>> + bool negate;
>> +
>> + /**
>> + * Clears the sign bit for floating point values, and computes the
>> integer
>> + * absolute value for integers. Note that the negate modifier acts
>> after
>> + * the absolute value modifier, therefore if both are set then all
>> inputs
>> + * will become negative.
>> + */
>> + bool abs;
>> + /*@}*/
>> +
>> + /**
>> + * For each input component, says which component of the register it
>> is
>> + * chosen from. Note that which elements of the swizzle are used and
>> which
>> + * are ignored are based on the write mask for most opcodes - for
>> example,
>> + * a statement like "foo.xzw = bar.zyx" would have a writemask of
>> 1101b and
>> + * a swizzle of {2, x, 1, 0} where x means "don't care."
>> + */
>> + uint8_t swizzle[4];
>> +} nir_alu_src;
>> +
>> +typedef struct {
>> + nir_dest dest;
>> +
>> + /**
>> + * \name saturate output modifier
>> + *
>> + * Only valid for opcodes that output floating-point numbers. Clamps
>> the
>> + * output to between 0.0 and 1.0 inclusive.
>> + */
>> +
>> + bool saturate;
>> +
>> + unsigned write_mask : 4; /* ignored if dest.is_ssa is true */
>> +} nir_alu_dest;
>> +
>> +#define OPCODE(name, num_inputs, per_component, output_size,
>> output_type, \
>> + input_sizes, input_types) \
>> + nir_op_##name,
>> +
>> +#define LAST_OPCODE(name) nir_last_opcode = nir_op_##name,
>> +
>> +typedef enum {
>> +#include "nir_opcodes.h"
>> + nir_num_opcodes = nir_last_opcode + 1
>> +} nir_op;
>> +
>> +#undef OPCODE
>> +#undef LAST_OPCODE
>> +
>> +typedef enum {
>> + nir_type_float,
>> + nir_type_int,
>> + nir_type_unsigned,
>> + nir_type_bool
>> +} nir_alu_type;
>> +
>> +typedef struct {
>> + const char *name;
>> +
>> + unsigned num_inputs;
>> +
>> + /**
>> + * If true, the opcode acts in the standard, per-component manner; the
>> + * operation is performed on each component (except the ones that are
>> masked
>> + * out) with the input being taken from the input swizzle for that
>> component.
>> + *
>> + * If false, the size of the output and inputs are explicitly given;
>> swizzle
>> + * and writemask are still in effect, but if the output component is
>> masked
>> + * out, then the input component may still be in use.
>> + *
>> + * The size of some of the inputs may be given (i.e. non-zero) even
>> though
>> + * per_component is false; in that case, each component of the input
>> acts
>> + * per-component, while the rest of the inputs and the output are
>> normal.
>> + * For example, for conditional select the condition is per-component
>> but
>> + * everything else is normal.
>> + */
>> + bool per_component;
>> +
>> + /**
>> + * If per_component is false, the number of components in the output.
>> + */
>> + unsigned output_size;
>> +
>> + /**
>> + * The type of vector that the instruction outputs. Note that this
>> + * determines whether the saturate modifier is allowed.
>> + */
>> +
>> + nir_alu_type output_type;
>> +
>> + /**
>> + * If per_component is false, the number of components in each input.
>> + */
>> + unsigned input_sizes[4];
>> +
>> + /**
>> + * The type of vector that each input takes. Note that negate is only
>> + * allowed on inputs with int or float type, and behaves differently
>> on the
>> + * two, and absolute value is only allowed on float type inputs.
>> + */
>> + nir_alu_type input_types[4];
>> +} nir_op_info;
>> +
>> +extern const nir_op_info nir_op_infos[nir_num_opcodes];
>> +
>> +typedef struct nir_alu_instr {
>> + nir_instr instr;
>> + nir_op op;
>> + bool has_predicate;
>> + nir_src predicate;
>> + nir_alu_dest dest;
>> + nir_alu_src src[];
>> +} nir_alu_instr;
>> +
>> +/* is this source channel used? */
>> +static inline bool
>> +nir_alu_instr_channel_used(nir_alu_instr *instr, unsigned src, unsigned
>> channel)
>> +{
>> + if (nir_op_infos[instr->op].input_sizes[src] > 0)
>> + return channel < nir_op_infos[instr->op].input_sizes[src];
>> +
>> + return (instr->dest.write_mask >> channel) & 1;
>>
>
> If per_component is false this should probably return true if any input
> channel is used?
per_component is dead as of 136/133, we just use the input/output sizes so
this is correct.
> +}
>> +
>> +typedef enum {
>> + nir_deref_type_var,
>> + nir_deref_type_array,
>> + nir_deref_type_struct
>> +} nir_deref_type;
>> +
>> +typedef struct nir_deref {
>> + nir_deref_type deref_type;
>> + struct nir_deref *child;
>> + const struct glsl_type *type;
>> +} nir_deref;
>> +
>> +typedef struct {
>> + nir_deref deref;
>> +
>> + nir_variable *var;
>> +} nir_deref_var;
>> +
>> +typedef struct {
>> + nir_deref deref;
>> +
>> + unsigned base_offset;
>> + bool has_indirect;
>> + nir_src indirect;
>> +} nir_deref_array;
>> +
>> +typedef struct {
>> + nir_deref deref;
>> +
>> + const char *elem;
>> +} nir_deref_struct;
>> +
>> +#define nir_deref_as_var(_deref) exec_node_data(nir_deref_var, _deref,
>> deref)
>> +#define nir_deref_as_array(_deref) \
>> + exec_node_data(nir_deref_array, _deref, deref)
>> +#define nir_deref_as_struct(_deref) \
>> + exec_node_data(nir_deref_struct, _deref, deref)
>> +
>> +typedef struct {
>> + nir_instr instr;
>> +
>> + unsigned num_params;
>> + nir_deref_var **params;
>> + nir_deref_var *return_deref;
>> +
>> + bool has_predicate;
>> + nir_src predicate;
>> +
>> + struct nir_function_overload *callee;
>> +} nir_call_instr;
>> +
>> +#define INTRINSIC(name, num_srcs, src_components, has_dest,
>> dest_components, \
>> + num_variables, num_indices, flags) \
>> + nir_intrinsic_##name,
>> +
>> +#define LAST_INTRINSIC(name) nir_last_intrinsic = nir_intrinsic_##name,
>> +
>> +typedef enum {
>> +#include "nir_intrinsics.h"
>> + nir_num_intrinsics = nir_last_intrinsic + 1
>> +} nir_intrinsic_op;
>> +
>> +#undef INTRINSIC
>> +#undef LAST_INTRINSIC
>> +
>> +typedef struct {
>> + nir_instr instr;
>> +
>> + nir_intrinsic_op intrinsic;
>> +
>> + nir_dest dest;
>> +
>> + int const_index[3];
>> +
>> + nir_deref_var *variables[2];
>> +
>> + bool has_predicate;
>> + nir_src predicate;
>> +
>> + nir_src src[];
>> +} nir_intrinsic_instr;
>> +
>> +/**
>> + * \name NIR intrinsics semantic flags
>> + *
>> + * information about what the compiler can do with the intrinsics.
>> + *
>> + * \sa nir_intrinsic_info::flags
>> + */
>> +/*@{*/
>> +/**
>> + * whether the intrinsic can be safely eliminated if none of its register
>> + * outputs are being used.
>> + */
>> +#define NIR_INTRINSIC_CAN_ELIMINATE (1 << 0)
>> +
>>
>
> Make these enums, easier for debugging.
>
> +/**
>> + * Whether the intrinsic can be reordered with respect to any other
>> intrinsic,
>> + * i.e. whether the only reodering dependencies of the intrinsic are due
>> to the
>>
>
> reordering
>
> Might want to note that even though an instruction itself is reorderable,
> it still may not cross another non-reorderable instruction such as a
> tessellation barrier instruction.
>
>
> + * register reads/writes.
>> + */
>> +#define NIR_INTRINSIC_CAN_REORDER (1 << 1)
>> +/*@}*/
>> +
>> +#define NIR_INTRINSIC_MAX_INPUTS 4
>> +
>> +typedef struct {
>> + const char *name;
>> +
>> + unsigned num_srcs; /** < number of register/SSA inputs */
>> +
>> + /** number of components of each input register */
>> + unsigned src_components[NIR_INTRINSIC_MAX_INPUTS];
>> +
>> + bool has_dest;
>> +
>> + /** number of components of each output register */
>> + unsigned dest_components;
>> +
>> + /** the number of inputs/outputs that are variables */
>> + unsigned num_variables;
>> +
>> + /** the number of constant indices used by the intrinsic */
>> + unsigned num_indices;
>> +
>> + /** semantic flags for calls to this intrinsic */
>> + unsigned flags;
>> +} nir_intrinsic_info;
>> +
>> +extern const nir_intrinsic_info nir_intrinsic_infos[nir_num_intrinsics];
>> +
>> +/**
>> + * \group texture information
>> + *
>> + * This gives semantic information about textures which is useful to the
>> + * frontend, the backend, and lowering passes, but not the optimizer.
>> + */
>> +
>> +typedef enum {
>> + nir_tex_src_coord,
>> + nir_tex_src_projector,
>> + nir_tex_src_comparitor, /* shadow comparitor */
>>
>
> comparator
thanks
>
>
> + nir_tex_src_offset,
>> + nir_tex_src_bias,
>> + nir_tex_src_lod,
>> + nir_tex_src_ms_index, /* MSAA sample index */
>> + nir_tex_src_ddx,
>> + nir_tex_src_ddy,
>> + nir_tex_src_sampler_index, /* < dynamically uniform indirect index */
>> + nir_num_texinput_types
>> +} nir_texinput_type;
>> +
>> +typedef enum {
>> + nir_texop_tex, /**< Regular texture look-up */
>> + nir_texop_txb, /**< Texture look-up with LOD bias */
>> + nir_texop_txl, /**< Texture look-up with explicit LOD
>> */
>> + nir_texop_txd, /**< Texture look-up with partial
>> derivatvies */
>> + nir_texop_txf, /**< Texel fetch with explicit LOD */
>> + nir_texop_txf_ms, /**< Multisample texture fetch */
>> + nir_texop_txs, /**< Texture size */
>> + nir_texop_lod, /**< Texture lod query */
>> + nir_texop_tg4, /**< Texture gather */
>> + nir_texop_query_levels /**< Texture levels query */
>> +} nir_texop;
>> +
>> +typedef struct {
>> + nir_instr instr;
>> +
>> + bool has_predicate;
>> + nir_src predicate;
>> +
>> + enum glsl_sampler_dim sampler_dim;
>> + nir_alu_type dest_type;
>> +
>> + nir_texop op;
>> + nir_dest dest;
>> + nir_src src[4];
>> + nir_texinput_type src_type[4];
>> + unsigned num_srcs, coord_components;
>>
>
> num_coord_components for consistency?
>
> + bool is_array, is_shadow;
>> +
>> + /**
>> + * If is_shadow is true, whether this is the old-style shadow that
>> outputs 4
>> + * components or the new-style shadow that outputs 1 component.
>> + */
>> + bool is_new_style_shadow;
>>
>
> is_four_component_shadow would be clearer, "new" will get old over the
> lifespan of this code...
Point.
>
> +
>> + /* constant offset - must be 0 if the offset source is used */
>> + int const_offset[4];
>> +
>> + /* gather component selector */
>> + unsigned component : 2;
>> +
>> + unsigned sampler_index;
>> + nir_deref_var *sampler; /* if this is NULL, use sampler_index instead
>> */
>> +} nir_tex_instr;
>> +
>> +static inline unsigned
>> +nir_tex_instr_dest_size(nir_tex_instr *instr)
>> +{
>> + if (instr->op == nir_texop_txs) {
>> + unsigned ret;
>> + switch (instr->sampler_dim) {
>> + case GLSL_SAMPLER_DIM_1D:
>> + case GLSL_SAMPLER_DIM_BUF:
>> + ret = 1;
>> + break;
>> + case GLSL_SAMPLER_DIM_2D:
>> + case GLSL_SAMPLER_DIM_CUBE:
>> + case GLSL_SAMPLER_DIM_MS:
>> + case GLSL_SAMPLER_DIM_RECT:
>> + case GLSL_SAMPLER_DIM_EXTERNAL:
>> + ret = 2;
>> + break;
>> + case GLSL_SAMPLER_DIM_3D:
>> + ret = 3;
>> + break;
>> + default:
>> + assert(0);
>> + break;
>> + }
>> + if (instr->is_array)
>> + ret++;
>> + return ret;
>> + }
>> +
>> + if (instr->op == nir_texop_query_levels)
>> + return 2;
>> +
>> + if (instr->is_shadow && instr->is_new_style_shadow)
>> + return 1;
>> +
>> + return 4;
>> +}
>> +
>> +static inline unsigned
>> +nir_tex_instr_src_size(nir_tex_instr *instr, unsigned src)
>> +{
>> + if (instr->src_type[src] == nir_tex_src_coord)
>> + return instr->coord_components;
>> +
>> +
>> + if (instr->src_type[src] == nir_tex_src_offset ||
>> + instr->src_type[src] == nir_tex_src_ddx ||
>> + instr->src_type[src] == nir_tex_src_ddy) {
>> + if (instr->is_array)
>> + return instr->coord_components - 1;
>> + else
>> + return instr->coord_components;
>>
>
> Please avoid mixing 0 and 1 indexing in the same variable if at all
> possible.
>
That's not really what's going on here. In GLSL, if you have an ND array
texture, it takes (N+1)D coordinates. You could make the case that NIR
should use an extra source for this, but there's precedent.
>
> + }
>> +
>> + return 1;
>> +}
>> +
>> +static inline int
>> +nir_tex_instr_src_index(nir_tex_instr *instr, nir_texinput_type type)
>> +{
>> + for (unsigned i = 0; i < instr->num_srcs; i++)
>> + if (instr->src_type[i] == type)
>> + return (int) i;
>> +
>> + return -1;
>> +}
>> +
>> +typedef struct {
>> + union {
>> + float f[4];
>> + int32_t i[4];
>> + uint32_t u[4];
>> + };
>> +} nir_const_value;
>> +
>>
>
> Matrix types have been lowered before these are constructed? Comparing
> with nir_constant_data which can fit a mat4.
Yes.
>
>
> +typedef struct {
>> + nir_instr instr;
>> +
>> + union {
>> + nir_const_value value;
>> + nir_const_value *array;
>> + };
>> +
>> + unsigned num_components;
>> +
>> + /**
>> + * The number of constant array elements to be copied into the
>> variable. If
>> + * this != 0, then value.array holds the array of size array_elems;
>> + * otherwise, value.value holds the single vector constant (the more
>> common
>> + * case, and the only case for SSA destinations).
>> + */
>> + unsigned array_elems;
>> +
>> + bool has_predicate;
>> + nir_src predicate;
>> +
>> + nir_dest dest;
>> +} nir_load_const_instr;
>> +
>> +typedef enum {
>> + nir_jump_return,
>> + nir_jump_break,
>> + nir_jump_continue,
>> +} nir_jump_type;
>> +
>> +typedef struct {
>> + nir_instr instr;
>> + nir_jump_type type;
>> +} nir_jump_instr;
>> +
>> +/* creates a new SSA variable in an undefined state */
>> +
>> +typedef struct {
>> + nir_instr instr;
>> + nir_ssa_def def;
>> +} nir_ssa_undef_instr;
>> +
>> +typedef struct {
>> + struct exec_node node;
>> + struct nir_block *pred;
>>
>
> A comment that pred here = predecessor.
>
Sure
> + nir_src src;
>> +} nir_phi_src;
>> +
>> +typedef struct {
>> + nir_instr instr;
>> +
>> + struct exec_list srcs;
>>
>
> Comment on real type of srcs. Same for all the exec_list/set/hash_table
> types in this file that don't already have such comments.
>
Probably a good idea.
>
> + nir_dest dest;
>> +} nir_phi_instr;
>> +
>> +#define nir_instr_as_alu(_instr) exec_node_data(nir_alu_instr, _instr,
>> instr)
>> +#define nir_instr_as_call(_instr) exec_node_data(nir_call_instr, _instr,
>> instr)
>> +#define nir_instr_as_jump(_instr) exec_node_data(nir_jump_instr, _instr,
>> instr)
>> +#define nir_instr_as_texture(_instr) \
>> + exec_node_data(nir_tex_instr, _instr, instr)
>>
>
> Note actual type name "tex" but function "texture". Slightly confusing.
Fixed in a later patch
>
>
> +#define nir_instr_as_intrinsic(_instr) \
>> + exec_node_data(nir_intrinsic_instr, _instr, instr)
>> +#define nir_instr_as_load_const(_instr) \
>> + exec_node_data(nir_load_const_instr, _instr, instr)
>> +#define nir_instr_as_ssa_undef(_instr) \
>> + exec_node_data(nir_ssa_undef_instr, _instr, instr)
>> +#define nir_instr_as_phi(_instr) \
>> + exec_node_data(nir_phi_instr, _instr, instr)
>> +
>> +
>> +/*
>> + * Control flow
>> + *
>> + * Control flow consists of a tree of control flow nodes, which include
>> + * if-statements and loops. The leaves of the tree are basic blocks,
>> lists of
>> + * instructions that always run start-to-finish. Each basic block also
>> keeps
>> + * track of its successors (blocks which may run immediately after the
>> current
>> + * block) and predecessors (blocks which could have run immediately
>> before the
>> + * current block). Each function also has a start block and an end block
>> which
>> + * all return statements point to (which is always empty). Together, all
>> the
>> + * blocks with their predecessors and successors make up the control flow
>> + * graph (CFG) of the function. There are helpers that modify the tree of
>> + * control flow nodes while modifying the CFG appropriately; these
>> should be
>> + * used instead of modifying the tree directly.
>> + */
>> +
>> +typedef enum {
>> + nir_cf_node_block,
>> + nir_cf_node_if,
>> + nir_cf_node_loop,
>> + nir_cf_node_function
>> +} nir_cf_node_type;
>> +
>> +typedef struct nir_cf_node {
>> + struct exec_node node;
>> + nir_cf_node_type type;
>> + struct nir_cf_node *parent;
>> +} nir_cf_node;
>> +
>> +typedef struct nir_block {
>> + nir_cf_node cf_node;
>> + struct exec_list instr_list;
>> +
>> + unsigned index;
>>
>
> Deserves a comment what this is
>
Sure
>
> +
>> + /*
>> + * Each block can only have up to 2 successors, so we put them in a
>> simple
>> + * array - no need for anything more complicated.
>> + */
>> + struct nir_block *successors[2];
>> +
>> + struct set *predecessors;
>>
>
> Why a set and not a simple array/list?
a) Ordering doesn't matter and that's explicit here. b) This gets modified
on-the-fly every time the CFG gets modified so we want quick
insertion/removal (not array) and c) a set really isn't that much more
complex than a linked list given that we already have the datastructure.
>
>
> +} nir_block;
>> +
>> +#define nir_block_first_instr(block) \
>> + exec_node_data(nir_instr, exec_list_get_head(&(block)->instr_list),
>> node)
>> +#define nir_block_last_instr(block) \
>> + exec_node_data(nir_instr, exec_list_get_tail(&(block)->instr_list),
>> node)
>> +
>> +#define nir_foreach_instr(block, instr) \
>> + foreach_list_typed(nir_instr, instr, node, &(block)->instr_list)
>> +#define nir_foreach_instr_reverse(block, instr) \
>> + foreach_list_typed_reverse(nir_instr, instr, node,
>> &(block)->instr_list)
>> +#define nir_foreach_instr_safe(block, instr) \
>> + foreach_list_typed_safe(nir_instr, instr, node, &(block)->instr_list)
>> +
>> +typedef struct {
>> + nir_cf_node cf_node;
>> + nir_src condition;
>> + struct exec_list then_list;
>> + struct exec_list else_list;
>> +} nir_if;
>> +
>> +#define nir_if_first_then_node(if) \
>> + exec_node_data(nir_cf_node, exec_list_get_head(&(if)->then_list),
>> node)
>> +#define nir_if_last_then_node(if) \
>> + exec_node_data(nir_cf_node, exec_list_get_tail(&(if)->then_list),
>> node)
>> +#define nir_if_first_else_node(if) \
>> + exec_node_data(nir_cf_node, exec_list_get_head(&(if)->else_list),
>> node)
>> +#define nir_if_last_else_node(if) \
>> + exec_node_data(nir_cf_node, exec_list_get_tail(&(if)->else_list),
>> node)
>> +
>> +typedef struct {
>> + nir_cf_node cf_node;
>> + struct exec_list body;
>> +} nir_loop;
>> +
>> +#define nir_loop_first_cf_node(loop) \
>> + exec_node_data(nir_cf_node, exec_list_get_head(&(loop)->body), node)
>> +#define nir_loop_last_cf_node(loop) \
>> + exec_node_data(nir_cf_node, exec_list_get_tail(&(loop)->body), node)
>> +
>> +typedef struct {
>> + nir_cf_node cf_node;
>> +
>> + /** pointer to the overload of which this is an implementation */
>> + struct nir_function_overload *overload;
>> +
>> + struct exec_list body; /** < list of nir_cf_node */
>> +
>> + nir_block *start_block, *end_block;
>> +
>> + /** list for all local variables in the function */
>> + struct exec_list locals;
>> +
>> + /** array of variables used as parameters */
>> + unsigned num_params;
>> + nir_variable **params;
>> +
>> + /** variable used to hold the result of the function */
>> + nir_variable *return_var;
>> +
>> + /** list of local registers in the function */
>> + struct exec_list registers;
>> +
>> + /** next available local register index */
>> + unsigned reg_alloc;
>> +
>> + /** next available SSA value index */
>> + unsigned ssa_alloc;
>> +
>> + /* total number of basic blocks, only valid when block_index_dirty =
>> false */
>> + unsigned num_blocks;
>> +
>> + bool block_index_dirty;
>> +} nir_function_impl;
>> +
>> +#define nir_cf_node_next(_node) \
>> + exec_node_data(nir_cf_node, exec_node_get_next(&(_node)->node), node)
>> +
>> +#define nir_cf_node_prev(_node) \
>> + exec_node_data(nir_cf_node, exec_node_get_prev(&(_node)->node), node)
>> +
>> +#define nir_cf_node_is_first(_node) \
>> + exec_node_is_head_sentinel((_node)->node.prev)
>> +
>> +#define nir_cf_node_is_last(_node) \
>> + exec_node_is_tail_sentinel((_node)->node.next)
>> +
>> +#define nir_cf_node_as_block(node) \
>> + exec_node_data(nir_block, node, cf_node)
>> +
>> +#define nir_cf_node_as_if(node) \
>> + exec_node_data(nir_if, node, cf_node)
>> +
>> +#define nir_cf_node_as_loop(node) \
>> + exec_node_data(nir_loop, node, cf_node)
>> +
>> +#define nir_cf_node_as_function(node) \
>> + exec_node_data(nir_function_impl, node, cf_node)
>> +
>> +typedef enum {
>> + nir_parameter_in,
>> + nir_parameter_out,
>> + nir_parameter_inout,
>> +} nir_parameter_type;
>> +
>> +typedef struct {
>> + nir_parameter_type param_type;
>> + const struct glsl_type *type;
>> +} nir_parameter;
>> +
>> +typedef struct nir_function_overload {
>> + struct exec_node node;
>> +
>> + unsigned num_params;
>> + nir_parameter *params;
>> + const struct glsl_type *return_type;
>> +
>> + nir_function_impl *impl; /** < NULL if the overload is only declared
>> yet */
>> +
>> + /** pointer to the function of which this is an overload */
>> + struct nir_function *function;
>> +} nir_function_overload;
>> +
>> +typedef struct nir_function {
>> + struct exec_node node;
>> +
>> + struct exec_list overload_list;
>> + const char *name;
>> +} nir_function;
>> +
>> +#define nir_function_first_overload(func) \
>> + exec_node_data(nir_function_overload, \
>> + exec_list_get_head(&(func)->overload_list), node)
>> +
>> +typedef struct nir_shader {
>> + /** hash table of name -> uniform */
>> + struct hash_table *uniforms;
>> +
>> + /** hash table of name -> input */
>> + struct hash_table *inputs;
>> +
>> + /** hash table of name -> output */
>> + struct hash_table *outputs;
>> +
>> + /** list of global variables in the shader */
>> + struct exec_list globals;
>> +
>> + struct exec_list system_values;
>> +
>> + struct exec_list functions;
>> +
>> + /** list of global registers in the shader */
>> + struct exec_list registers;
>> +
>> + /** structures used in this shader */
>> + unsigned num_user_structures;
>> + struct glsl_type **user_structures;
>> +
>> + /** next available global register index */
>> + unsigned reg_alloc;
>> +} nir_shader;
>> +
>> +#define nir_foreach_overload(shader, overload) \
>> + foreach_list_typed(nir_function, func, node, &(shader)->functions) \
>> + foreach_list_typed(nir_function_overload, overload, node, \
>> + &(func)->overload_list)
>> +
>> +#ifdef __cplusplus
>> +} /* extern "C" */
>> +#endif
>>
>
> ... snipped for brevity, no comments on remaining files of patch, LGTM
>
>
> Above is really only nitpicks, ignore at will. Kudos for Connor/Jason for
> NIR, very impressive!
>
> Note i have reviewed the dd1f3c71f2d5253a69469f55cfa53dc3e5793b80 version
> at
> http://cgit.freedesktop.org/~jekstrand/mesa/tree/src/glsl/
> nir/nir.h?h=review/nir-v1
> rather than the exact version sent out to ml.
>
> Patch 004 is
> Reviewed-By glenn.kennard <glenn.kennard at gmail.com>
>
Thanks. I think I've addressed most of your comments. I decided to leave
some of it alone for now. We've got quite a few patches on the list at
this point and the little fixups can wait.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141219/13a9fcd0/attachment-0001.html>
More information about the mesa-dev
mailing list