[Mesa-dev] [Bug 105670] [regression][hang] Trine1EE hangs GPU after loading screen on Mesa3D-17.3 and later

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105670

--- Comment #11 from Gert Wollny  ---
The if statement can be be true because  

   tmp0.w = (R3.w >= 0.0 ? abs(ps_lc18.w) : abs(ps_lc18.y));

with ps_lc18.y = -1 and ps_lc18.w = 0.0, and then 

   R3.w = (tmp0.w);
   R3.w = (R5.w * R3.w);

which means at this point R3.w can be zero.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 15/19] nir: use load_local_group_size

[Jason Ekstrand]

On March 24, 2018 19:34:05 Rob Clark  wrote:

On Sat, Mar 24, 2018 at 8:12 PM, Jason Ekstrand  wrote:
On March 24, 2018 16:24:57 Rob Clark  wrote:

On Fri, Mar 23, 2018 at 4:59 PM, Jason Ekstrand 
wrote:
On Fri, Mar 23, 2018 at 1:35 PM, Karol Herbst  wrote:

On Fri, Mar 23, 2018 at 9:18 PM, Jason Ekstrand 
wrote:
On Fri, Mar 23, 2018 at 12:33 PM, Karol Herbst 
wrote:

From: Rob Clark 

If local_size is not known at compile time, which is the case with
clover, use the load_local_group_size intrinsic instead.

Signed-off-by: Karol Herbst 
---
src/compiler/nir/nir_lower_system_values.c | 25
+
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/compiler/nir/nir_lower_system_values.c
b/src/compiler/nir/nir_lower_system_values.c
index d507c28f421..ff4e09c8e61 100644
--- a/src/compiler/nir/nir_lower_system_values.c
+++ b/src/compiler/nir/nir_lower_system_values.c
@@ -57,19 +57,28 @@ convert_block(nir_block *block, nir_builder *b)
*"The value of gl_GlobalInvocationID is equal to
*gl_WorkGroupID * gl_WorkGroupSize +
gl_LocalInvocationID"
*/
+ nir_ssa_def *local_size_def;

- nir_const_value local_size;
- memset(_size, 0, sizeof(local_size));
- local_size.u64[0] = b->shader->info.cs.local_size[0];
- local_size.u64[1] = b->shader->info.cs.local_size[1];
- local_size.u64[2] = b->shader->info.cs.local_size[2];
+ /* if local_size[] is already known, use that, otherwise use
+  * load_local_group_size intrinsic:
+  */
+ if (b->shader->info.cs.local_size[0]) {
+nir_const_value local_size;
+memset(_size, 0, sizeof(local_size));
+local_size.u64[0] = b->shader->info.cs.local_size[0];
+local_size.u64[1] = b->shader->info.cs.local_size[1];
+local_size.u64[2] = b->shader->info.cs.local_size[2];
+
+local_size_def = nir_build_imm(b, 3, bit_size,
local_size);

+ } else {
+local_size_def = nir_load_local_group_size(b, bit_size);
+ }


I commented on an earlier patch about how the approach to building the
32/64-bit immediates is wrong.

oh right, I totally forgot about that.

Setting that aside, this patch looks fine to me in principal.  There's a
part of me that doesn't like using cs.local_size[0] being the trigger
but I
think it's probably ok.  Maybe we should assert that cs_local_size is
either
all zero (second case) or all not zero (first case) just to be safe.

I think the main problem here is, that even with OpenCL kernels you
can specify it, but then overwrite it at runtime again. So yes I
agree, that we need something better here.


Oh, that's tricky then.  We could make nir_lower_system_values take a flag
or OpenCL callers could just whack it all to 0 after spirv_to_nir.c.  Or you
could do recompiles or something.

I'm half-way towards thinking we should have MESA_SHADER_KERNEL vs
MESA_SHADER_COMPUTE to deal with cl/cuda/etc vs glsl compute shaders..

So far, I've been trying to avoid that, but maybe it would be a better
solution..

I think that would be a bit short-sighted.  There are cases where that might
be a good idea but I doubt this is one of them. It may will be that someone
will decide that runtime local sizes would make a great Vulkan extension and
then we'd be switching on API for no good reason.

hmm, the extension case is a good counter-argument (and probably also
applies in the other cases I was thinking of but am too jet-lagged to
remember)..

I guess until someone comes up with a better idea, sticking to
info.cs.local_size[0]==0 => use intrinsic seems sane.. a
local_size[0]==0 is impossible.  Extra asserts for local_size[1..2]==0
is a good idea

Fine by me



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 15/19] nir: use load_local_group_size

[Rob Clark]

On Sat, Mar 24, 2018 at 8:12 PM, Jason Ekstrand  wrote:
> On March 24, 2018 16:24:57 Rob Clark  wrote:
>
> On Fri, Mar 23, 2018 at 4:59 PM, Jason Ekstrand 
> wrote:
> On Fri, Mar 23, 2018 at 1:35 PM, Karol Herbst  wrote:
>
> On Fri, Mar 23, 2018 at 9:18 PM, Jason Ekstrand 
> wrote:
> On Fri, Mar 23, 2018 at 12:33 PM, Karol Herbst 
> wrote:
>
> From: Rob Clark 
>
> If local_size is not known at compile time, which is the case with
> clover, use the load_local_group_size intrinsic instead.
>
> Signed-off-by: Karol Herbst 
> ---
> src/compiler/nir/nir_lower_system_values.c | 25
> +
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/compiler/nir/nir_lower_system_values.c
> b/src/compiler/nir/nir_lower_system_values.c
> index d507c28f421..ff4e09c8e61 100644
> --- a/src/compiler/nir/nir_lower_system_values.c
> +++ b/src/compiler/nir/nir_lower_system_values.c
> @@ -57,19 +57,28 @@ convert_block(nir_block *block, nir_builder *b)
> *"The value of gl_GlobalInvocationID is equal to
> *gl_WorkGroupID * gl_WorkGroupSize +
> gl_LocalInvocationID"
> */
> + nir_ssa_def *local_size_def;
>
> - nir_const_value local_size;
> - memset(_size, 0, sizeof(local_size));
> - local_size.u64[0] = b->shader->info.cs.local_size[0];
> - local_size.u64[1] = b->shader->info.cs.local_size[1];
> - local_size.u64[2] = b->shader->info.cs.local_size[2];
> + /* if local_size[] is already known, use that, otherwise use
> +  * load_local_group_size intrinsic:
> +  */
> + if (b->shader->info.cs.local_size[0]) {
> +nir_const_value local_size;
> +memset(_size, 0, sizeof(local_size));
> +local_size.u64[0] = b->shader->info.cs.local_size[0];
> +local_size.u64[1] = b->shader->info.cs.local_size[1];
> +local_size.u64[2] = b->shader->info.cs.local_size[2];
> +
> +local_size_def = nir_build_imm(b, 3, bit_size,
> local_size);
>
> + } else {
> +local_size_def = nir_load_local_group_size(b, bit_size);
> + }
>
>
> I commented on an earlier patch about how the approach to building the
> 32/64-bit immediates is wrong.
>
> oh right, I totally forgot about that.
>
> Setting that aside, this patch looks fine to me in principal.  There's a
> part of me that doesn't like using cs.local_size[0] being the trigger
> but I
> think it's probably ok.  Maybe we should assert that cs_local_size is
> either
> all zero (second case) or all not zero (first case) just to be safe.
>
> I think the main problem here is, that even with OpenCL kernels you
> can specify it, but then overwrite it at runtime again. So yes I
> agree, that we need something better here.
>
>
> Oh, that's tricky then.  We could make nir_lower_system_values take a flag
> or OpenCL callers could just whack it all to 0 after spirv_to_nir.c.  Or you
> could do recompiles or something.
>
> I'm half-way towards thinking we should have MESA_SHADER_KERNEL vs
> MESA_SHADER_COMPUTE to deal with cl/cuda/etc vs glsl compute shaders..
>
> So far, I've been trying to avoid that, but maybe it would be a better
> solution..
>
> I think that would be a bit short-sighted.  There are cases where that might
> be a good idea but I doubt this is one of them. It may will be that someone
> will decide that runtime local sizes would make a great Vulkan extension and
> then we'd be switching on API for no good reason.
>

hmm, the extension case is a good counter-argument (and probably also
applies in the other cases I was thinking of but am too jet-lagged to
remember)..

I guess until someone comes up with a better idea, sticking to
info.cs.local_size[0]==0 => use intrinsic seems sane.. a
local_size[0]==0 is impossible.  Extra asserts for local_size[1..2]==0
is a good idea.

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 15/19] nir: use load_local_group_size

[Jason Ekstrand]

On March 24, 2018 16:24:57 Rob Clark  wrote:

On Fri, Mar 23, 2018 at 4:59 PM, Jason Ekstrand  wrote:
On Fri, Mar 23, 2018 at 1:35 PM, Karol Herbst  wrote:

On Fri, Mar 23, 2018 at 9:18 PM, Jason Ekstrand 
wrote:
On Fri, Mar 23, 2018 at 12:33 PM, Karol Herbst 
wrote:

From: Rob Clark 

If local_size is not known at compile time, which is the case with
clover, use the load_local_group_size intrinsic instead.

Signed-off-by: Karol Herbst 
---
src/compiler/nir/nir_lower_system_values.c | 25
+
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/compiler/nir/nir_lower_system_values.c
b/src/compiler/nir/nir_lower_system_values.c
index d507c28f421..ff4e09c8e61 100644
--- a/src/compiler/nir/nir_lower_system_values.c
+++ b/src/compiler/nir/nir_lower_system_values.c
@@ -57,19 +57,28 @@ convert_block(nir_block *block, nir_builder *b)
*"The value of gl_GlobalInvocationID is equal to
*gl_WorkGroupID * gl_WorkGroupSize +
gl_LocalInvocationID"
*/
+ nir_ssa_def *local_size_def;

- nir_const_value local_size;
- memset(_size, 0, sizeof(local_size));
- local_size.u64[0] = b->shader->info.cs.local_size[0];
- local_size.u64[1] = b->shader->info.cs.local_size[1];
- local_size.u64[2] = b->shader->info.cs.local_size[2];
+ /* if local_size[] is already known, use that, otherwise use
+  * load_local_group_size intrinsic:
+  */
+ if (b->shader->info.cs.local_size[0]) {
+nir_const_value local_size;
+memset(_size, 0, sizeof(local_size));
+local_size.u64[0] = b->shader->info.cs.local_size[0];
+local_size.u64[1] = b->shader->info.cs.local_size[1];
+local_size.u64[2] = b->shader->info.cs.local_size[2];
+
+local_size_def = nir_build_imm(b, 3, bit_size,
local_size);

+ } else {
+local_size_def = nir_load_local_group_size(b, bit_size);
+ }


I commented on an earlier patch about how the approach to building the
32/64-bit immediates is wrong.

oh right, I totally forgot about that.

Setting that aside, this patch looks fine to me in principal.  There's a
part of me that doesn't like using cs.local_size[0] being the trigger
but I
think it's probably ok.  Maybe we should assert that cs_local_size is
either
all zero (second case) or all not zero (first case) just to be safe.

I think the main problem here is, that even with OpenCL kernels you
can specify it, but then overwrite it at runtime again. So yes I
agree, that we need something better here.


Oh, that's tricky then.  We could make nir_lower_system_values take a flag
or OpenCL callers could just whack it all to 0 after spirv_to_nir.c.  Or you
could do recompiles or something.

I'm half-way towards thinking we should have MESA_SHADER_KERNEL vs
MESA_SHADER_COMPUTE to deal with cl/cuda/etc vs glsl compute shaders..

So far, I've been trying to avoid that, but maybe it would be a better
solution..

I think that would be a bit short-sighted.  There are cases where that 
might be a good idea but I doubt this is one of them. It may will be that 
someone will decide that runtime local sizes would make a great Vulkan 
extension and then we'd be switching on API for no good reason.


--Jason


BR,
-R

I think this looks good for now and we can let OpenCL users of NIR whack it
to 0.  It's a fairly obvious behavior of "if you don't have it, load it" and
we can let the CL driver sort out how they want to handle recompiles.




nir_ssa_def *group_id = nir_load_work_group_id(b, bit_size);
nir_ssa_def *local_id = nir_load_local_invocation_id(b,
bit_size);

- sysval = nir_iadd(b, nir_imul(b, group_id,
-   nir_build_imm(b, 3, bit_size,
local_size)),
-  local_id);
+ sysval = nir_iadd(b, nir_imul(b, group_id, local_size_def),
+   local_id);
break;
}

--
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 0/8] freedreno: a2xx improvements

[Rob Clark]

On Fri, Mar 23, 2018 at 7:21 AM, Wladimir J. van der Laan
 wrote:
> While working on a205 support for i.MX51/53, I've also written some patches
> that are not specific to a20x but should apply to the whole a2xx range.
>
> As I'm figuring out how to handle backward compatibility to other a2xx, I
> think it makes sense to send these upstream already to reduce the patch stack.
>
> Changes since v1:
>
> - Split up rnndb patch into a patch that changes formate numeration
>   and one that changes BLEND->BLEND2.
> - fd2_emit emit_texture const correctness.
>
> I checked that there is no Gallium capability to be set for TEXTURE_RECT.
>
> Changes since v2:
> - Tabs/spaces cleanup patch 6,7,8
>

Patchset looks reasonable to me, from a quick look.  Testing a22x
isn't so easy without resurrecting an ancient downstream kernel for
old snadragon devices, so I think it is ok to ignore that.  If the
rnndb bits are merged already, then:

Reviewed-by: Rob Clark 

(and if not ping me, I might have overlooked some patches..)


> Wladimir J. van der Laan (8):
>   freedreno: a2xx: Update rnndb header for formats enumeration
>   freedreno: a2xx: Change use of BLEND_ to BLEND2_
>   freedreno: a2xx: Fix fd2_tex_swiz
>   freedreno: a2xx: Prevent crash in emit_texture if view is not set
>   freedreno: a2xx: Support TEXTURE_RECT
>   freedreno: a2xx: Compressed textures support
>   freedreno: a2xx: implement SEQ/SNE instructions
>   freedreno: a2xx: Implement DP2 instruction
>
>  src/gallium/drivers/freedreno/a2xx/a2xx.xml.h | 33 +++-
>  src/gallium/drivers/freedreno/a2xx/fd2_compiler.c | 47 
> +--
>  src/gallium/drivers/freedreno/a2xx/fd2_emit.c | 13 +--
>  src/gallium/drivers/freedreno/a2xx/fd2_gmem.c |  4 +-
>  src/gallium/drivers/freedreno/a2xx/fd2_util.c | 29 +-
>  src/gallium/drivers/freedreno/a2xx/ir-a2xx.c  |  1 +
>  src/gallium/drivers/freedreno/a2xx/ir-a2xx.h  |  1 +
>  7 files changed, 90 insertions(+), 38 deletions(-)
>
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 15/19] nir: use load_local_group_size

[Rob Clark]

On Fri, Mar 23, 2018 at 4:59 PM, Jason Ekstrand  wrote:
> On Fri, Mar 23, 2018 at 1:35 PM, Karol Herbst  wrote:
>>
>> On Fri, Mar 23, 2018 at 9:18 PM, Jason Ekstrand 
>> wrote:
>> > On Fri, Mar 23, 2018 at 12:33 PM, Karol Herbst 
>> > wrote:
>> >>
>> >> From: Rob Clark 
>> >>
>> >> If local_size is not known at compile time, which is the case with
>> >> clover, use the load_local_group_size intrinsic instead.
>> >>
>> >> Signed-off-by: Karol Herbst 
>> >> ---
>> >>  src/compiler/nir/nir_lower_system_values.c | 25
>> >> +
>> >>  1 file changed, 17 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/src/compiler/nir/nir_lower_system_values.c
>> >> b/src/compiler/nir/nir_lower_system_values.c
>> >> index d507c28f421..ff4e09c8e61 100644
>> >> --- a/src/compiler/nir/nir_lower_system_values.c
>> >> +++ b/src/compiler/nir/nir_lower_system_values.c
>> >> @@ -57,19 +57,28 @@ convert_block(nir_block *block, nir_builder *b)
>> >>*"The value of gl_GlobalInvocationID is equal to
>> >>*gl_WorkGroupID * gl_WorkGroupSize +
>> >> gl_LocalInvocationID"
>> >>*/
>> >> + nir_ssa_def *local_size_def;
>> >>
>> >> - nir_const_value local_size;
>> >> - memset(_size, 0, sizeof(local_size));
>> >> - local_size.u64[0] = b->shader->info.cs.local_size[0];
>> >> - local_size.u64[1] = b->shader->info.cs.local_size[1];
>> >> - local_size.u64[2] = b->shader->info.cs.local_size[2];
>> >> + /* if local_size[] is already known, use that, otherwise use
>> >> +  * load_local_group_size intrinsic:
>> >> +  */
>> >> + if (b->shader->info.cs.local_size[0]) {
>> >> +nir_const_value local_size;
>> >> +memset(_size, 0, sizeof(local_size));
>> >> +local_size.u64[0] = b->shader->info.cs.local_size[0];
>> >> +local_size.u64[1] = b->shader->info.cs.local_size[1];
>> >> +local_size.u64[2] = b->shader->info.cs.local_size[2];
>> >> +
>> >> +local_size_def = nir_build_imm(b, 3, bit_size,
>> >> local_size);
>> >>
>> >> + } else {
>> >> +local_size_def = nir_load_local_group_size(b, bit_size);
>> >> + }
>> >
>> >
>> > I commented on an earlier patch about how the approach to building the
>> > 32/64-bit immediates is wrong.
>> >
>>
>> oh right, I totally forgot about that.
>>
>> > Setting that aside, this patch looks fine to me in principal.  There's a
>> > part of me that doesn't like using cs.local_size[0] being the trigger
>> > but I
>> > think it's probably ok.  Maybe we should assert that cs_local_size is
>> > either
>> > all zero (second case) or all not zero (first case) just to be safe.
>> >
>>
>> I think the main problem here is, that even with OpenCL kernels you
>> can specify it, but then overwrite it at runtime again. So yes I
>> agree, that we need something better here.
>
>
> Oh, that's tricky then.  We could make nir_lower_system_values take a flag
> or OpenCL callers could just whack it all to 0 after spirv_to_nir.c.  Or you
> could do recompiles or something.
>

I'm half-way towards thinking we should have MESA_SHADER_KERNEL vs
MESA_SHADER_COMPUTE to deal with cl/cuda/etc vs glsl compute shaders..

So far, I've been trying to avoid that, but maybe it would be a better
solution..

BR,
-R

> I think this looks good for now and we can let OpenCL users of NIR whack it
> to 0.  It's a fairly obvious behavior of "if you don't have it, load it" and
> we can let the CL driver sort out how they want to handle recompiles.
>
>>
>> >>
>> >>
>> >>   nir_ssa_def *group_id = nir_load_work_group_id(b, bit_size);
>> >>   nir_ssa_def *local_id = nir_load_local_invocation_id(b,
>> >> bit_size);
>> >>
>> >> - sysval = nir_iadd(b, nir_imul(b, group_id,
>> >> -   nir_build_imm(b, 3, bit_size,
>> >> local_size)),
>> >> -  local_id);
>> >> + sysval = nir_iadd(b, nir_imul(b, group_id, local_size_def),
>> >> +   local_id);
>> >>   break;
>> >>}
>> >>
>> >> --
>> >> 2.14.3
>> >>
>> >> ___
>> >> mesa-dev mailing list
>> >> mesa-dev@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> >
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 06/19] RFC: nir/vtn: "raw" pointer support

[Rob Clark]

On Fri, Mar 23, 2018 at 5:18 PM, Jason Ekstrand  wrote:
> On Fri, Mar 23, 2018 at 2:15 PM, Karol Herbst  wrote:
>>
>> On Fri, Mar 23, 2018 at 10:07 PM, Jason Ekstrand 
>> wrote:
>> > +list
>> >
>> > On Fri, Mar 23, 2018 at 1:45 PM, Karol Herbst 
>> > wrote:
>> >>
>> >> On Fri, Mar 23, 2018 at 9:30 PM, Jason Ekstrand 
>> >> wrote:
>> >> > As I've been rewriting core NIR deref handling, I've been thinking
>> >> > about
>> >> > this problem quite a bit.  One objective I have is to actually make
>> >> > UBO
>> >> > and
>> >> > SSBO access go through derefs instead of just being an offset and
>> >> > index
>> >> > so
>> >> > that the compiler can better reason about them.  In particular, I
>> >> > want
>> >> > to be
>> >> > able to start doing load/store elimination on SSBOs, SLM, and
>> >> > whatever
>> >> > CL
>> >> > has which would be great for everyone's compute performance (GL,
>> >> > Vulkan,
>> >> > CL,
>> >> > etc.).
>> >> >
>> >> > I would be lying if I said I had a full plan but I do have part of a
>> >> > plan.
>> >> > In my patch which adds the deref instructions, I add a new "cast"
>> >> > deref
>> >> > type
>> >> > which takes an arbitrary value as it's source and kicks out a deref
>> >> > with
>> >> > a
>> >> > type.  Whenever we discover that the source of the cast is actually
>> >> > another
>> >> > deref which is compatible (same type etc.), copy propagation gets rid
>> >> > of
>> >> > the
>> >> > cast for you.  The idea is that, instead of doing a
>> >> > load_raw(raw_ptr),
>> >> > you
>> >> > would do a load((type *)raw_ptr).
>> >> >
>> >> > Right now, most of the core NIR optimizations will throw a fit if
>> >> > they
>> >> > ever
>> >> > see a cast.  This is intentional because it requires us to manually
>> >> > go
>> >> > through and handle casts.  This would mean that, at the moment, you
>> >> > would
>> >> > have to lower to load_raw intrinsics almost immediately after coming
>> >> > out
>> >> > of
>> >> > SPIR-V.
>> >> >
>> >>
>> >> Well it gets more fun with OpenCL 2.0 where you can have generic
>> >> pointer where you only know the type at creation type. You can also
>> >> declare generic pointers as function inputs in a way, that you never
>> >> actually know from where you have to load if you only have that one
>> >> function. So the actual load operation depends on when you create the
>> >> initial pointer variable (you can cast from X to generic, but not the
>> >> other way around).
>> >>
>> >> Which in the end means you can end up with load(generic_ptr) and only
>> >> following the chain up to it's creation (with function inlining in
>> >> mind) you know the actual memory target.
>> >
>> >
>> > Yup.  And there will always be crazy cases where you can't actually
>> > follow
>> > it and you have to emit a pile of code to load different ways depending
>> > on
>> > some bits somewhere that tell you how to load it.  I'm well aware of the
>> > insanity. :-)  This is part of the reason why I'm glad I'm not trying to
>> > write an OpenCL 2.0 driver.
>> >
>> > This insanity is exactly why I'm suggesting the pointer casting.  Sure,
>> > you
>> > may not know the data type until the actual load.  In that case, you end
>> > up
>> > with the cast being right before the load.  If you don't know the
>> > storage
>> > class, maybe you have to switch and do multiple casts based on some
>> > bits.
>> > Alternatively, if you don't know the storage class, we can just let the
>> > deref mode be 0 for "I don't know". or maybe multiple bits for "these
>> > are
>> > the things it might be".  In any case, I think we can handle it.
>> >
>>
>> there shouldn't be a situation where we don't know, except when you
>> don't inline all functions. I think Rob had the idea of fat pointers
>> where a pointer is a vec2 and the 2nd component contains the actual
>> pointer type and you end up with a switch over the type to get the
>> correct storage class. And if the compiler inlines all functions, it
>> should be able to optimize that switch away.
>
>
> Right.  Today, we live in a world where all functions are inlined.  Sadly, I
> fear that world may come to and end one of these days. :(
>

fwiw, so far I'm mostly caring about the inline-all-the-fxns case..

for the cases where we don't know what sort of pointer we have, Karol
(iirc?) suggested name-mangling functions, which seems semi-sane.. but
I've mostly tried to ignore that for now until we have more basic
things working.

Possibly we need a compiler option to lower everything to
load/store_global (or maybe "raw" is a better name?) for hw that can
remap local memory into a single address space and use the same
load/store instructions.  I think that should be at least enough to
move forward with nv hw + fxn calls.  Less so for intel/adreno but
from my PoV I'm willing to solve that problem later.

BR,
-R
___

[Mesa-dev] [Bug 105670] [regression][hang] Trine1EE hangs GPU after loading screen on Mesa3D-17.3 and later

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105670

--- Comment #10 from i...@yahoo.com ---
(In reply to Gert Wollny from comment #9)
> Actually, 
>   if (R3.w != -R3.w) 
> will never fail, because R3.w = R0.w = (ps_lc17.y) = 1.0;

You are correct for the "if" with the "break".

However my question still stands for the other "if (R3.w != -R3.w)" right
before the "break" one.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question about min_index/max_index calculation

[Connor Abbott]

On Sat, Mar 24, 2018 at 6:00 PM, Jason Ekstrand  wrote:
> On Sat, Mar 24, 2018 at 2:27 PM, Marek Olšák  wrote:
>>
>> On Sat, Mar 24, 2018 at 1:36 PM, Connor Abbott 
>> wrote:
>>>
>>> If Gallium was being lazy and not
>>> specifying the bounds for internal shaders, that needs to be fixed for
>>> the HW to work properly.
>>
>>
>> I don't understand the sentence. Shaders don't interact with vertex
>> indices. I also don't like the word "lazy". The proper expression is "saving
>> time".
>
>
> I figured he meant for things like u_blitter.  But why those things would be
> using an index buffer is beyond me...

Yeah, that was just speculation. I just meant to explain why Mali
might require the min/max index up-front, unlike other HW (AFAIK). I'm
certainly no expert when it comes to Gallium, and I don't have the
hardware to reproduce the problem.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question about min_index/max_index calculation

[Jason Ekstrand]

On Sat, Mar 24, 2018 at 2:27 PM, Marek Olšák  wrote:

> On Sat, Mar 24, 2018 at 1:36 PM, Connor Abbott 
> wrote:
>
>> If Gallium was being lazy and not
>> specifying the bounds for internal shaders, that needs to be fixed for
>> the HW to work properly.
>>
>
> I don't understand the sentence. Shaders don't interact with vertex
> indices. I also don't like the word "lazy". The proper expression is
> "saving time".
>

I figured he meant for things like u_blitter.  But why those things would
be using an index buffer is beyond me...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question about min_index/max_index calculation

[Marek Olšák]

On Sat, Mar 24, 2018 at 5:27 PM, Marek Olšák  wrote:

> On Sat, Mar 24, 2018 at 1:36 PM, Connor Abbott 
> wrote:
>
>> If Gallium was being lazy and not
>> specifying the bounds for internal shaders, that needs to be fixed for
>> the HW to work properly.
>>
>
> I don't understand the sentence. Shaders don't interact with vertex
> indices. I also don't like the word "lazy". The proper expression is
> "saving time".
>

Here is how to do it:

if (max_index != ~0u)
   // index bounds are valid;
else
   // scan the index buffer manually;

u_vbuf_get_minmax_index can be used for the scanning.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question about min_index/max_index calculation

[Marek Olšák]

On Sat, Mar 24, 2018 at 1:36 PM, Connor Abbott  wrote:

> If Gallium was being lazy and not
> specifying the bounds for internal shaders, that needs to be fixed for
> the HW to work properly.
>

I don't understand the sentence. Shaders don't interact with vertex
indices. I also don't like the word "lazy". The proper expression is
"saving time".

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] nvc0/ir: fix INTERP_* with indirect inputs

[Ilia Mirkin]

There were two problems, both of which are fixed now:
 - The indirect address was not being shifted by 4
 - The indirect address was being placed as an argument in the offset case

This fixes some of the new interpolateAt* piglits which now test for
these situations.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp 
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index 09b5228127a..3c5bad05fe7 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -3083,10 +3083,11 @@ Converter::handleINTERP(Value *dst[4])
  assert(sym[c]);
  op = insn->op;
  mode = insn->ipa;
+ ptr = insn->getIndirect(0, 0);
   }
} else {
   if (src.isIndirect(0))
- ptr = fetchSrc(src.getIndirect(0), 0, NULL);
+ ptr = shiftAddress(fetchSrc(src.getIndirect(0), 0, NULL));
 
   // We can assume that the fixed index will point to an input of the same
   // interpolation type in case of an indirect.
@@ -3144,10 +3145,10 @@ Converter::handleINTERP(Value *dst[4])
   insn = mkOp1(op, TYPE_F32, dst[c], sym[c] ? sym[c] : srcToSym(src, c));
   if (op == OP_PINTERP)
  insn->setSrc(1, w);
-  if (ptr)
- insn->setIndirect(0, 0, ptr);
   if (offset)
  insn->setSrc(op == OP_PINTERP ? 2 : 1, offset);
+  if (ptr)
+ insn->setIndirect(0, 0, ptr);
 
   insn->setInterpolate(mode);
}
-- 
2.16.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105731] linker error "fragment shader input ... has no matching output in the previous stage" when previous stage's output declaration in a separate shader object

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105731

Marcel Heinz  changed:

   What|Removed |Added

 CC||quisqui...@gmx.de

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105731] linker error "fragment shader input ... has no matching output in the previous stage" when previous stage's output declaration in a separate shader object

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105731

Bug ID: 105731
   Summary: linker error "fragment shader input ... has no
matching output in the previous stage" when previous
stage's output declaration in a separate shader object
   Product: Mesa
   Version: git
  Hardware: All
OS: Linux (All)
Status: NEW
  Severity: minor
  Priority: medium
 Component: glsl-compiler
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: quisqui...@gmx.de
QA Contact: intel-3d-b...@lists.freedesktop.org

Created attachment 138342
  --> https://bugs.freedesktop.org/attachment.cgi?id=138342=edit
C/OpenGL code to trigger the issue

GLSL spec (using version 4.30 here, section "4.3.4 Input variables", page 43f)
states that there should be no link error if an input variable is used for
which there is no static use in the previous stage, but a valid output
declaration.

Mesa's linker seems to adhere to this not in every case. If the declaration is
in a separate shader object, it fails to link. See the attached example code.
The key point is the following vertex shader:

static const char *vs_part1=
"out vec4 foo;\n"
"void unused() {foo=vec4(1);}\n";

static const char *vs_part2=
"in vec4 pos;\n"
"void main() { gl_Position = pos; }\n";

used in conjunction with this fragment shader:
static const char *fs_part1=
"in vec4 foo;\n"
"out vec4 color;\n"
"void main() {color=foo;}\n";

When this is created as a single GL_VERTEX_SHADER with both strings
concatenated (via glShaderSource), the resulting program will link, just with a
warning

warning: fragment shader varying foo not written by vertex shader

which is conforming behavior.

However, when vs_part1 and vs_part2 are compiled as separate shader objects,
the final link fails with

error: fragment shader input `foo' has no matching output in the previous 
   stage

which (in my interpretation of the spec) should not happen.

Tested with:
OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) Haswell Mobile 
OpenGL core profile version string: 4.5 (Core Profile) Mesa 17.1.1
(git-092c485)

as well as:
OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) Haswell Mobile 
OpenGL core profile version string: 4.5 (Core Profile) Mesa 18.1.0-devel
(git-ff0e3fa1fe)

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Question about min_index/max_index calculation

[Connor Abbott]

My understanding is that unlike most other architectures, Mali does
vertex shading on every vertex up-front, completely ignoring the index
buffer. Primitive assembly and tile binning happen after every vertex
is transformed. There is no cache of transformed vertices. Utgard also
only supports GLES2, so the index buffer data is always passed through
a CPU pointer. Since it's possible to calculate the bounds on the CPU
without stalling, and since the HW was designed with low transistor
count in mind, they don't compute the bounds on the HW, and instead
expect you to pass it through. If Gallium was being lazy and not
specifying the bounds for internal shaders, that needs to be fixed for
the HW to work properly. Or, we need to guess by looking at the vertex
buffer size as Ilia mentioned.

On Sun, Mar 18, 2018 at 12:59 AM, Marek Olšák  wrote:
> The index bounds are computed only when they are needed for uploading
> vertices that are passed via a CPU pointer (user_buffer). In all other
> cases, computing the index bounds has a performance cost, which can be very
> significant.
>
> If you rely on u_vbuf to upload vertices for you, you shouldn't need the
> index bounds.
>
> Marek
>
> On Sat, Mar 17, 2018 at 2:12 PM, Erico Nunes  wrote:
>>
>> Hi all,
>>
>> I have been working to add indexed drawing/glDrawElements support to
>> the mesa-lima driver currently in development
>> (https://github.com/yuq/mesa-lima).
>> For that implementation, it seems that we need to have the minimum and
>> maximum index values from the index buffer available in order to set
>> up a proper command stream.
>> Mesa provides these values in pipe_draw_info.min_index and
>> pipe_draw_info.max_index, however in some cases we noticed that it
>> decides to not calculate those. This happens because of
>> st_context.draw_needs_minmax_index being false after evaluating the
>> vertex data. In those cases, min_index gets to be 0 and max_index gets
>> to be 0x.
>> According to the gallium documentation, this seems to be on purpose
>> and apparently drivers should be able to handle the 0 and 0x
>> case and be able to render anyway. However, we haven't figured out a
>> way to do the render anyway with 0 and 0x.
>>
>> For us it would be interesting to always have mesa calculate those
>> values for indexed drawing. We haven't been able to figure out a way
>> to do that without changing the mesa generic code. Is there some way
>> we could accomplish that in driver specific code?
>> Otherwise, can you provide some advice on how to best handle this?
>>
>> Using mesa 17.3 and kmscube with the following patch is one way to
>> reproduce st_context.draw_needs_minmax_index not being set.
>>
>> https://gist.githubusercontent.com/enunes/366398fbee3d194deb3a46ef9c2ca78d/raw/82a2c8084236e35635b7a247609213d0068974e3/kmscube.patch
>> The only way that this works for us with the current implementation is
>> by hacking st_context.draw_needs_minmax_index to be always true in
>> some way.
>>
>> Thanks
>>
>> Erico
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105730] [llvmpipe] 116 piglit failures, 19197 crashes on ppc (ppc64, mesa-18.0.0_rc5)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105730

--- Comment #2 from erhar...@mailbox.org ---
Created attachment 138339
  --> https://bugs.freedesktop.org/attachment.cgi?id=138339=edit
html summary from 'pigllit run all'

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105730] [llvmpipe] 116 piglit failures, 19197 crashes on ppc (ppc64, mesa-18.0.0_rc5)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105730

--- Comment #1 from erhar...@mailbox.org ---
Created attachment 138338
  --> https://bugs.freedesktop.org/attachment.cgi?id=138338=edit
log of the crashes (via journalctl -k)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105730] [llvmpipe] 116 piglit failures, 19197 crashes on ppc (ppc64, mesa-18.0.0_rc5)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105730

Bug ID: 105730
   Summary: [llvmpipe] 116 piglit failures, 19197 crashes on ppc
(ppc64, mesa-18.0.0_rc5)
   Product: Mesa
   Version: git
  Hardware: PowerPC
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: erhar...@mailbox.org
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 138337
  --> https://bugs.freedesktop.org/attachment.cgi?id=138337=edit
results from 'pigllit run all'

My G5 had some fun running piglit with llvmpipe on Big Endian ppc64. Just
wanted to share the results.

# glxinfo | grep -i opengl
OpenGL vendor string: VMware, Inc.
OpenGL renderer string: llvmpipe (LLVM 5.0, 128 bits)
OpenGL core profile version string: 3.3 (Core Profile) Mesa 18.0.0-rc5
OpenGL core profile shading language version string: 3.30
OpenGL core profile context flags: (none)
OpenGL core profile profile mask: core profile
OpenGL core profile extensions:
OpenGL version string: 3.0 Mesa 18.0.0-rc5
OpenGL shading language version string: 1.30
OpenGL context flags: (none)
OpenGL extensions:
OpenGL ES profile version string: OpenGL ES 3.0 Mesa 18.0.0-rc5
OpenGL ES profile shading language version string: OpenGL ES GLSL ES 3.00
OpenGL ES profile extensions:

# ./piglit run -s sanity results/sanity
[1/1] pass: 1  
Thank you for running Piglit!
Results have been written to /root/build/piglit/results/sanity

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are the QA Contact for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105670] [regression][hang] Trine1EE hangs GPU after loading screen on Mesa3D-17.3 and later

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105670

--- Comment #9 from Gert Wollny  ---
Actually, 
  if (R3.w != -R3.w) 
will never fail, because R3.w = R0.w = (ps_lc17.y) = 1.0;

The compiler should optimize this away, and in my attempts to create a piglit
it always did so far.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 105670] [regression][hang] Trine1EE hangs GPU after loading screen on Mesa3D-17.3 and later

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105670

--- Comment #8 from i...@yahoo.com ---
Created attachment 138335
  --> https://bugs.freedesktop.org/attachment.cgi?id=138335=edit
Fragment/Pixel Shader generated by Wine3.3, extracted from lookup of #55251 op
in the trace.

It's quite simple to use qapitrace to get all needed information. Just go to
op#55251, right click and select "lookup", it would start `glretrace` and dump
all information about current state, including the shaders, textures etc.

On unrelated to this bug note, this condition looks really fishy to me:
   if (R3.w != -R3.w) break;
Will this condition ever fail?

If variables were two's complement integers, the above line would have been
false if `R3.w==0` (or INT_MIN). However with float numbers we can have
separate +0.0 and -0.0, just like we have +inf and -inf.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev