<div dir="ltr">+Matt, +Ken </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Apr 12, 2017 at 6:09 PM, Boyan Ding <<a href="mailto:boyan.j.ding@gmail.com" target="_blank">boyan.j.ding@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">2017-04-13 2:25 GMT+08:00 Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>>: > On Wed, Apr 12, 2017 at 6:14 AM, Boyan Ding <<a href="mailto:boyan.j.ding@gmail.com">boyan.j.ding@gmail.com</a>> 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: <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a> >> Signed-off-by: Boyan Ding <<a href="mailto:boyan.j.ding@gmail.com">boyan.j.ding@gmail.com</a>> >> --- >> 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. > </div></div>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 <div class="HOEnZb"><div class="h5"> >> 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 >> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > </div></div></blockquote></div> </div>