[Mesa-dev] [PATCH 004/133] nir: add the core datastructures
Connor Abbott
cwabbott0 at gmail.com
Tue Dec 16 15:07:08 PST 2014
On Tue, Dec 16, 2014 at 5:48 PM, Eric Anholt <eric at anholt.net> wrote:
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
>> 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
>
> Just some comment comments. I've been happy with NIR in my usage of it
> in VC4. So much nicer than TGSI, GLSL IR, or Mesa IR.
>
> With the trivial stuff fixed, this and patches 1-3 are:
>
> Reviewed-by: Eric Anholt <eric at anholt.net>
>
>> 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
>
>> +/**
>> + * Data stored in an nir_constant
>> + */
>> +union nir_constant_data {
>> + unsigned u[16];
>> + int i[16];
>> + float f[16];
>> + bool b[16];
>> +};
>
> Funny indentation
>
>> +
>> +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;
>
> The ir_instruction? I suspect some updating here is appropriate.
>
>> +
>> + /* Array elements / Structure Fields */
>> + struct nir_constant **elements;
>> +} nir_constant;
>
>> +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;
>
> ir_variable_interpolation doesn't exist, perhaps just point to
> INTERP_QUALIFIER_NONE instead.
>
>
>> +typedef struct {
>> + struct exec_node node;
>> +
>> + unsigned num_components; /** < number of vector components */
>> + unsigned num_array_elems; /** < size of array (0 for no array) */
>
> "0 for non-array" instead?
>
>> +
>> + /** 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 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;
>
> Aren't these TODOs resolved by the du/ud chains being in the
> nir_register?
No, those only list all the definitions and all the uses, whereas a
hash set here would tell use which definitions each use can pick up
and which use each definition can reach (AKA the UD and DU chains).
That being said, the plan is to do all the optimizations in SSA, so
unless someone has a burning need to do optimizations in non-SSA NIR
it's pretty useless.
>
>> +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;
>
> "For inputs interpreted as floating point, flips the sign bit. For
> inputs interpreted as integers, performs the two's complement negation."
>
>> +
>> + /**
>> + * 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 {
>> + 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];
>
> I thought abs was allowed on ints, too?
Yeah, I think I changed my mind on that but forgot to update the comment.
>
>> +} nir_op_info;
>
>
>> +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;
>
> FWIW, I found the .const_index vs .src vs .variables usage in
> nir_intrinsic_instr confusing, and some docs would be nice. Basically I
> just cargo-culted intrinsic emit stuff from glsl_to_nir and codegen
> stuff from brw.
Hmm, maybe saying src, var, and idx might be easier... src[] is named
for consistency with nir_alu_instr, and I called it src there since
accessing it is a pretty frequent operation in NIR so the name should
be as short as possible.
>
>> +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;
>
> Each output register? Isn't there only one?
Yeah, originally there was support for more than one but I removed it
since there are no known cases in GLSL where we'd want more than one
output value.
>
>> +
>> + /** 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;
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list