[Mesa-dev] [PATCH v2 17/25] spirv: add support for doubles to OpSpecConstant

Jason Ekstrand jason at jlekstrand.net
Wed Jan 4 14:45:27 UTC 2017


On Jan 4, 2017 4:54 AM, "Erik Faye-Lund" <kusmabite at gmail.com> wrote:

On Jan 3, 2017 16:34, "Jason Ekstrand" <jason at jlekstrand.net> wrote:

On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romero <jasuarez at igalia.com>
wrote:

> From: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/amd/vulkan/radv_pipeline.c    |  5 +++-
>  src/compiler/spirv/nir_spirv.h    |  5 +++-
>  src/compiler/spirv/spirv_to_nir.c | 51 ++++++++++++++++++++++++++++++
> +++++----
>  src/intel/vulkan/anv_pipeline.c   |  5 +++-
>  4 files changed, 58 insertions(+), 8 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline
> .c
> index 23ed2d2..6d8299e 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -188,7 +188,10 @@ radv_shader_compile_to_nir(struct radv_device
> *device,
>                                 assert(data + entry.size <=
> spec_info->pData + spec_info->dataSize);
>
>                                 spec_entries[i].id =
> spec_info->pMapEntries[i].constantID;
> -                               spec_entries[i].data = *(const uint32_t
> *)data;
> +                                if (spec_info->dataSize == 8)
> +                                   spec_entries[i].data64 = *(const
> uint64_t *)data;
> +                                else
> +                                   spec_entries[i].data32 = *(const
> uint32_t *)data;
>

radv is silly and uses tabs. :(  You used spaces...


>                         }
>                 }
>
> diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv
> .h
> index 500f2cb..33a2781 100644
> --- a/src/compiler/spirv/nir_spirv.h
> +++ b/src/compiler/spirv/nir_spirv.h
> @@ -38,7 +38,10 @@ extern "C" {
>
>  struct nir_spirv_specialization {
>     uint32_t id;
> -   uint32_t data;
> +   union {
> +      uint32_t data32;
> +      uint64_t data64;
> +   };
>  };
>
>  nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 2a60b53..380fbae 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -31,6 +31,14 @@
>  #include "nir/nir_constant_expressions.h"
>  #include "spirv_info.h"
>
> +struct spec_constant_value {
> +   bool is_double;
> +   union {
> +      uint32_t data32;
> +      uint64_t data64;
> +   };
> +};
> +
>  void
>  _vtn_warn(const char *file, int line, const char *msg, ...)
>  {
> @@ -942,11 +950,14 @@ spec_constant_decoration_cb(struct vtn_builder *b,
> struct vtn_value *v,
>     if (dec->decoration != SpvDecorationSpecId)
>        return;
>
> -   uint32_t *const_value = data;
> +   struct spec_constant_value *const_value = data;
>
>     for (unsigned i = 0; i < b->num_specializations; i++) {
>        if (b->specializations[i].id == dec->literals[0]) {
> -         *const_value = b->specializations[i].data;
> +         if (const_value->is_double)
> +            const_value->data64 = b->specializations[i].data64;
> +         else
> +            const_value->data32 = b->specializations[i].data32;
>           return;
>        }
>     }
> @@ -956,8 +967,22 @@ static uint32_t
>  get_specialization(struct vtn_builder *b, struct vtn_value *val,
>                     uint32_t const_value)
>  {
> -   vtn_foreach_decoration(b, val, spec_constant_decoration_cb,
> &const_value);
> -   return const_value;
> +   struct spec_constant_value data;
> +   data.is_double = false;
> +   data.data32 = const_value;
> +   vtn_foreach_decoration(b, val, spec_constant_decoration_cb, &data);
> +   return data.data32;
> +}
> +
> +static uint64_t
> +get_specialization64(struct vtn_builder *b, struct vtn_value *val,
> +                   uint64_t const_value)
> +{
> +   struct spec_constant_value data;
> +   data.is_double = true;
> +   data.data64 = const_value;
> +   vtn_foreach_decoration(b, val, spec_constant_decoration_cb, &data);
> +   return data.data64;
>  }
>
>  static void
> @@ -1016,10 +1041,26 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>        }
>        break;
>     }
> -   case SpvOpSpecConstant:
> +   case SpvOpSpecConstant: {
>        assert(glsl_type_is_scalar(val->const_type));
>        val->constant->values[0].u32[0] = get_specialization(b, val, w[3]);
> +      int bit_size = glsl_get_bit_size(val->const_type);
> +      if (bit_size == 64) {
> +         union {
> +            uint64_t u64;
> +            struct {
> +               uint32_t u1;
> +               uint32_t u2;
> +            };
> +         } di;
> +         di.u1 = w[3];
> +         di.u2 = w[4];
>

You could also just do *(const uint64_t *)&w[3].  Might be easier.



That would violate strict-aliasing.


Grr... You're right.  It's read-only so it probably wouldn't matter but it
does violate the rule.  Maybe the thing to do would be to add a quick
helper:

uint64_t
vtn_u64_literal(uint64_t *w)
{
   return (uint64_t)w[1] << 32 | w[0];
}

+         val->constant->values[0].u64[0] = get_specialization64(b, val,
> di.u64);
> +      } else {
> +         val->constant->values[0].u32[0] = get_specialization(b, val,
> w[3]);
> +      }
>        break;
> +   }
>     case SpvOpSpecConstantComposite:
>     case SpvOpConstantComposite: {
>        unsigned elem_count = count - 3;
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index 9104267..30ac19a 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -117,7 +117,10 @@ anv_shader_compile_to_nir(struct anv_device *device,
>           assert(data + entry.size <= spec_info->pData +
> spec_info->dataSize);
>
>           spec_entries[i].id = spec_info->pMapEntries[i].constantID;
> -         spec_entries[i].data = *(const uint32_t *)data;
> +         if (spec_info->dataSize == 8)
> +            spec_entries[i].data64 = *(const uint64_t *)data;
> +         else
> +            spec_entries[i].data32 = *(const uint32_t *)data;
>        }
>     }
>
> --
> 2.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


_______________________________________________
mesa-dev mailing list
mesa-dev at lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170104/68ff6b39/attachment.html>


More information about the mesa-dev mailing list