[Mesa-dev] [PATCH] nir: Destination component count of shader_clock intrinsic is 2

[Jason Ekstrand]

+Matt, +Ken

On Wed, Apr 12, 2017 at 6:09 PM, Boyan Ding <boyan.j.ding at gmail.com> wrote:

> 2017-04-13 2:25 GMT+08:00 Jason Ekstrand <jason at jlekstrand.net>:
> > On Wed, Apr 12, 2017 at 6:14 AM, Boyan Ding <boyan.j.ding at gmail.com>
> wrote:
> >>
> >> This fixes the following error when using ARB_shader_clock on i965:
> >>         vec1 32 ssa_0 = intrinsic shader_clock () () ()
> >>         intrinsic store_var (ssa_0) (clock_retval) (3) /* wrmask=xy */
> >> error: src->ssa->num_components == num_components
> (nir/nir_validate.c:204)
> >>
> >> Cc: mesa-stable at lists.freedesktop.org
> >> Signed-off-by: Boyan Ding <boyan.j.ding at gmail.com>
> >> ---
> >>  src/compiler/glsl/glsl_to_nir.cpp | 3 ++-
> >>  src/compiler/nir/nir_intrinsics.h | 2 +-
> >>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> >> b/src/compiler/glsl/glsl_to_nir.cpp
> >> index f0557f985b..870d457681 100644
> >> --- a/src/compiler/glsl/glsl_to_nir.cpp
> >> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> >> @@ -930,7 +930,8 @@ nir_visitor::visit(ir_call *ir)
> >>           nir_builder_instr_insert(&b, &instr->instr);
> >>           break;
> >>        case nir_intrinsic_shader_clock:
> >> -         nir_ssa_dest_init(&instr->instr, &instr->dest, 1, 32, NULL);
> >> +         nir_ssa_dest_init(&instr->instr, &instr->dest, 2, 32, NULL);
> >> +         instr->num_components = 2;
> >
> >
> > This made me go look at the spec, and things get a bit more subtle...  In
> > particular, ARB_shader_clock specifies two builtin functions:
> >
> >         uvec2 clock2x32ARB(void);
> >         uint64_t clockARB(void);
> >
> >  Where the second one only exists if you support int64.  On gen8+, we do
> > support int64...
> >
> > My feeling is that the correct way to implement this is to make the NIR
> > intrinsic return a 64bit value and wrap it in a nir_unpack_64_2x32 if the
> > client asks for the 2x32 version.  If that's too much refactoring for
> you,
> > then this patch is probably sufficient to solve the issue today.
> >
>
> I agree with you. I'm not very familiar with nir internals, and was
> just copying TGSI's handling here. There will be more intrinsics with
> 64bit results, for example, ballot, which radv guys might be
> interested in.
>
> I won't mind if someone comes up with a better solution and replaces
> mine. But just as you said above, it solves the issue today. It's up
> to you to decide.
>
> Cheers,
> Boyan Ding
>
> >>           nir_builder_instr_insert(&b, &instr->instr);
> >>           break;
> >>        case nir_intrinsic_store_ssbo: {
> >> diff --git a/src/compiler/nir/nir_intrinsics.h
> >> b/src/compiler/nir/nir_intrinsics.h
> >> index 105c56f759..3a519a73dd 100644
> >> --- a/src/compiler/nir/nir_intrinsics.h
> >> +++ b/src/compiler/nir/nir_intrinsics.h
> >> @@ -91,7 +91,7 @@ BARRIER(memory_barrier)
> >>   * The latter can be used as code motion barrier, which is currently
> not
> >>   * feasible with NIR.
> >>   */
> >> -INTRINSIC(shader_clock, 0, ARR(0), true, 1, 0, 0, xx, xx, xx,
> >> NIR_INTRINSIC_CAN_ELIMINATE)
> >> +INTRINSIC(shader_clock, 0, ARR(0), true, 2, 0, 0, xx, xx, xx,
> >> NIR_INTRINSIC_CAN_ELIMINATE)
> >>
> >>  /*
> >>   * Memory barrier with semantics analogous to the compute shader
> >> --
> >> 2.12.0
> >>
> >> _______________________________________________
> >> 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/20170414/588e13e4/attachment-0001.html>