<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Fri, Mar 16, 2018 at 11:28 AM, Dylan Baker <<a href="mailto:dylan@pnwbakers.com" target="_blank">dylan@pnwbakers.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Quoting Rob Clark (2018-03-15 18:43:28) <div><div class="h5"> > diff --git a/src/compiler/nir/nir_intrinsics.py b/src/compiler/nir/nir_intrinsics.py > new file mode 100644 > index 00000000000..6bb6603586e > --- /dev/null > +++ b/src/compiler/nir/nir_intrinsics.py > @@ -0,0 +1,547 @@ > +# > +# Copyright (C) 2018 Red Hat > +# Copyright (C) 2014 Intel Corporation > +# > +# 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. > +# > + > +# This file defines all the available intrinsics in one place. > +# > +# The Intrinsic class corresponds one-to-one with nir_intrinsic_info > +# structure. > + > +class Intrinsic(object): > + """Class that represents all the information about an intrinsic opcode. > + NOTE: this must be kept in sync with nir_intrinsic_info. > + """ > + def __init__(self, name, src_components, dest_components, num_variables, > + indices, flags): > + """Parameters: > + > + - name: the intrinsic name > + - src_components: list of the number of components per src, 0 means > + vectorized instruction with number of components given in the > + num_components field in nir_intrinsic_instr. > + - dest_components: number of destination components, -1 means no > + dest, 0 means number of components given in num_components field > + in nir_intrinsic_instr. > + - num_variables: the number of variables > + - indices: list of constant indicies > + - flags: list of semantic flags > + """ > + assert isinstance(name, str) > + assert isinstance(src_components, list) > + if len(src_components) > 0: </div></div>if src_components: > + assert isinstance(src_components[0], int) > + assert isinstance(dest_components, int) > + assert isinstance(num_variables, int) > + assert isinstance(indices, list) > + if len(indices) > 0: ditto > + assert isinstance(indices[0], str) > + assert isinstance(flags, list) > + if len(flags) > 0: ditto <div><div class="h5"> > + assert isinstance(flags[0], str) > + > + <a href="http://self.name" rel="noreferrer" target="_blank">self.name</a> = name > + self.num_srcs = len(src_components) > + self.src_components = src_components > + self.has_dest = (dest_components >= 0) > + self.dest_components = dest_components > + self.num_variables = num_variables > + self.num_indices = len(indices) > + self.indices = indices > + self.flags = flags > + > +# > +# Possible indices: > +# > + > +# A constant 'base' value that is added to an offset src: > +BASE = "NIR_INTRINSIC_BASE" > +# For store instructions, a writemask: > +WRMASK = "NIR_INTRINSIC_WRMASK" > +# The stream-id for GS emit_vertex/end_primitive intrinsics: > +STREAM_ID = "NIR_INTRINSIC_STREAM_ID" > +# The clip-plane id for load_user_clip_plane intrinsics: > +UCP_ID = "NIR_INTRINSIC_UCP_ID" > +# The amount of data, starting from BASE, that this instruction > +# may access. This is used to provide bounds if the offset is > +# not constant. > +RANGE = "NIR_INTRINSIC_RANGE" > +# The vulkan descriptor set binding for vulkan_resource_index > +# intrinsic > +DESC_SET = "NIR_INTRINSIC_DESC_SET" > +# The vulkan descriptor set binding for vulkan_resource_index > +# intrinsic > +BINDING = "NIR_INTRINSIC_BINDING" > +# Component offset > +COMPONENT = "NIR_INTRINSIC_COMPONENT" > +# Interpolation mode (only meaningful for FS inputs) > +INTERP_MODE = "NIR_INTRINSIC_INTERP_MODE" > +# A binary nir_op to use when performing a reduction or scan operation > +REDUCTION_OP = "NIR_INTRINSIC_REDUCTION_OP" > +# Cluster size for reduction operations > +CLUSTER_SIZE = "NIR_INTRINSIC_CLUSTER_SIZE" > + > +# > +# Possible flags: > +# > + > +CAN_ELIMINATE = "NIR_INTRINSIC_CAN_ELIMINATE" > +CAN_REORDER = "NIR_INTRINSIC_CAN_REORDER" > + > +intr_opcodes = {} > + > +def intrinsic(name, src_comp=[], dest_comp=-1, num_vars=0, indices=[], flags=[]): > + assert name not in intr_opcodes > + intr = Intrinsic(name, src_comp, dest_comp, num_vars, indices, flags) > + intr_opcodes[name] = intr > + return intr </div></div>Please either remove the return statement (since it's unused in so many cases), or, (and this is my preference), can we do something like: intr_opcodes = { 'nop': Intrinsic('nop', flags=[CAN_ELIMINATE]), ... } I prefer this since each dictionary is clearly created without a function obscuring what's actually going on. If you dislike having to repeat the name you could even do something like: intr_opcodes = [ 'nop': Intrinsic('nop', flags=[CAN_ELIMINATE]), ... ] intr_opcodes = {<a href="http://i.name" rel="noreferrer" target="_blank">i.name</a>: i for i in intr_opcodes} </blockquote><div> </div><div>I'm not sure what I think about this. On the one hand, having the dictionary explicitly declared is nice. On the other hand, in nir_opcodes.py we have a bunch of other helper functions we declare along the way to help with specific kinds of opcodes. It's not as practical to do this if everything is inside of a dictionary declaration. </div><div>--Jason </div></div></div></div>