[Mesa-dev] [PATCH v0] nir: mako all the intrinsics

[Jason Ekstrand]

On Fri, Mar 16, 2018 at 11:28 AM, Dylan Baker <dylan at pnwbakers.com> wrote:

> Quoting Rob Clark (2018-03-15 18:43:28)
>
> > 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:
>
> 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
>
> > +           assert isinstance(flags[0], str)
> > +
> > +       self.name = 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
>
> 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 = {i.name: i for i in intr_opcodes}
>

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.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180316/28de9b67/attachment.html>