[Mesa-dev] [PATCH v2 17/25] spirv: add support for doubles to OpSpecConstant
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Thu Jan 5 06:27:44 UTC 2017
On Wed, 2017-01-04 at 16:17 +0100, Erik Faye-Lund wrote:
>
>
> On Jan 4, 2017 14:45, "Jason Ekstrand" <jason at jlekstrand.net> wrote:
> 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 igal
> ia.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];
> }
>
> A helper like this would indeed be nice (and seems more appropriate
> wrt endianness)!
>
Thanks for catching this! I will add this helper to the v3 of this
patch.
Sam
>
> > + 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
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170105/fabef313/attachment.sig>
More information about the mesa-dev
mailing list