Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables

[Thomas Helland]

Den ons. 28. nov. 2018 kl. 10:23 skrev Timothy Arceri :
>
> On 28/11/18 6:52 pm, Thomas Helland wrote:
> > Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri 
> > :
> >>
> >> This adds allows loop analysis to detect inductions varibales that
> >> are incremented in both branches of an if rather than in a main
> >> loop block. For example:
> >>
> >> loop {
> >>block block_1:
> >>/* preds: block_0 block_7 */
> >>vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
> >>vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
> >>vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
> >>vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
> >>vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
> >>vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
> >>vec1 32 ssa_14 = ige ssa_8, ssa_5
> >>/* succs: block_2 block_3 */
> >>if ssa_14 {
> >>   block block_2:
> >>   /* preds: block_1 */
> >>   break
> >>   /* succs: block_8 */
> >>} else {
> >>   block block_3:
> >>   /* preds: block_1 */
> >>   /* succs: block_4 */
> >>}
> >>block block_4:
> >>/* preds: block_3 */
> >>vec1 32 ssa_15 = ilt ssa_6, ssa_8
> >>/* succs: block_5 block_6 */
> >>if ssa_15 {
> >>   block block_5:
> >>   /* preds: block_4 */
> >>   vec1 32 ssa_16 = iadd ssa_8, ssa_7
> >>   vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
> >>   /* succs: block_7 */
> >>} else {
> >>   block block_6:
> >>   /* preds: block_4 */
> >>   vec1 32 ssa_18 = iadd ssa_8, ssa_7
> >>   vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
> >>   /* succs: block_7 */
> >>}
> >>block block_7:
> >>/* preds: block_5 block_6 */
> >>vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
> >>vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
> >>vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
> >>/* succs: block_1 */
> >> }
> >>
> >> Unfortunatly GCM could move the addition out of the if for us
> >> (making this patch unrequired) but we still cannot enable the GCM
> >> pass without regressions.
> >>
> >
> > Just some questions / suggestions from my side for now.
> > I'll try to take a closer look at the patch later today.
> >
> > While GCM would be nice, to me it seems that adding an
> > if-opt instead, that pulls common code from both branches
> > of an if out of the if on a more general basis, would get us
> > this, plus a bunch of other benefits? As far as I can see there
> > should never be negative impacts from pulling common code
> > out like that, but I might be wrong. Did you look into that?
> > I bet out did, I'm just interested in how that worked out.
>
> I didn't attempt this because pulling code out of the ifs can increase
> register pressure. This is one of the problems we have with the GCM pass
> currently, so for now I chose a more conservative approach.
>

Yeah, of course. I'm being dumb. It looks better in source code,
but as long as it does not lead to other optimizations it will only
cause the live range of the add to intersect with that of the branch
condition. The same amount of instructions will be executed
either way.

> >
> > Since GCM is not yet where we want it to be, maybe we'd
> > want to implement LICM? That obviously does not come
> > into play with what this patch adresses, but it might help
> > get a more accurate estimate of the cost/benefit of unrolling?
> > (Invariant computations that will be CSE'd will not be
> > counted multiple times). This might already be accounted
> > for by counting the invariant computations only once?
>
> No we don't do anything like this currently. The GCM pass can pull
> things out of loops also, but again we hit register pressure issues with
> that pass.
>
> As far as I'm aware reducing invariants is not where we get most of our
> wins from with unrolling. Removing indirect array access, improving
> opportunities for constant folding (and a bunch of other passes), being
> able to evaluate the unfolded loop with the surrounding code etc all
> result in greater benefits.
>
> With the limits we place on making sure we don't unroll large loops that
> are going to cause register use issues, nobody has yet been able to show
> that always unrolling loops is causing any harm, and it's certainly been
> shown to help :)

Thanks for taking the time with my stupidity =) I'll try to take a look at
these patches later tonight =)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 04/11] st/mesa: Use Array._DrawVAO in st_atom_array.c.

[Mathias Fröhlich]

Good Morning,

On Tuesday, 27 November 2018 10:17:07 CET Erik Faye-Lund wrote:
> On Tue, 2018-11-27 at 07:11 +0100, Mathias Fröhlich wrote:
> > Hi Erik,
> > 
> > > > On Monday, 26 November 2018 19:39:50 CET Erik Faye-Lund wrote:
> > > > > I know this is *very* late notice, but this commit broke Super
> > > > > Tux
> > > > > Kart
> > > > > on VirGL. Both the player-models as as well as the level data
> > > > > renders
> > > > > with gibberish vertex positions since this commit.
> > > > > 
> > > > > The fix that Rob Clark did on top does not fix the problem (and
> > > > > shouldn't have; VirGL doesn't use NIR).
> > > > 
> > > > Do you have any idea how I can reproduce that issue with the
> > > > least
> > > > effort?
> > > > 
> > > 
> > > Sadly, no. I run a qemu VM where I run super tux cart. It's a
> > > rather
> > > convoluted setup, I'm afraid. If you're interested in that Robert
> > > Foss
> > > has written an article about how to set something like this up
> > > here:
> > > https://memcpy.io/virtualizing-gpu-access.html
> > > 
> > > ...But I totally understand if this is asking a bit too much. I can
> > > help out with any information you need...
> > 
> > Thanks!
> > That, just means that looking into has to wait at least until the
> > weekend.
> > Probably even later.
> > 
> > And thanks for looking up the constants.
> > The effective binding computation depends on these and may change
> > the set up combined buffer objects. So these are interesting to know.
> > 
> > I have been putting a lot of internal verification into the code
> > paths
> > especially _mesa_update_vao_derived_arrays contains a larger
> > #ifndef NDEBUG part that may tell us if there is something
> > unexpected.
> > 
> > I assume you did run also with asserts enabled in the build?
> 
> Yes, I ran with asserts on, and none triggered.

Ok, there should not be a problem.
At least nothing that I anticipated goes wrong.

> > I can observe some flicker in supertuxcart on i965. The nvidia blob
> > seems
> > not to flicker here. Also when running through valgrind I don't get
> > that flicker
> > on i965. Is that flashing - initially looked like a lighting effect
> > of the game to
> > me - what you observe too?
> 
> No, the models are completely garbled. You can find some example
> screenshots here:
> 
> https://gitlab.freedesktop.org/virgl/virglrenderer/issues/59
> 
> > Also what are the game options? Are shaders enabled in some way?
> 
> I'm playing with the default settings. I'm not sure what you mean with
> "are shaders enabled"; VirGL is a gallium-driver, everything uses
> shaders.

Ok, that is about what I meant. Sometimes that goes back to a pre shader
render engine behind the scenens.
Yes, gallium is always shaders, but depending on the higher level render 
techinque used
you may need different array setups with more or less vertex attributes. Thing
of tangent space that you do not need for a simple renderer. So, finally that 
may
influence the setup and trigger things. But what you report sounds pretty
fundamental.

The good side is that I set up at least what was easy to set up here, that is
a fedora 29 using virgl on a fedora 28 host using an Intel skylake type GPU.

Linux 4.19.4-300.fc29.x86_64 #1 SMP Fri Nov 23 13:03:11 UTC 2018 x86_64
[info   ] IrrDriver: OpenGL version: 3.3
[info   ] IrrDriver: OpenGL vendor: Red Hat
[info   ] IrrDriver: OpenGL renderer: virgl
[info   ] IrrDriver: OpenGL version string: 3.3 (Core Profile) Mesa 18.2.4

But, that just works. supertuxkart runs without any vertex corruption problems
on that combination. The installed default rpm of mesa is not patched in any way
that makes me suspicious regarding our problem. And git log mesa-18.2.4 tells
me that the patch you mention is included in 18.2.4.

Means either I do not yet reproduce the problem correctly on the application 
side.
Well, seems like already the initial screen to configure the track and that
should show problems, which run already fine on my combination.

Or we have a side effect somewhere in the complete chain down to the
host system, which is triggered by that patch.

So, looking at that game directly on the host system, that one has flaws
like the mentioned flashlight like frames that are probably missing
some geometry in one way or the other.

Means here:
game + mesa-18.0.5 + Skylake GT2 -> fail
game + mesa-18.2.4 + virgl + mesa-18.0.5 + Skylake GT2 -> works

Hmm, one question, on the mentioned setup on virgl. How does
glxgears render on that setup? Or alternatively how do other OpenGL
applications different from supertuxkart on that setup?

best

Mathias


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

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Timothy Arceri]

On Thu, Nov 29, 2018, at 7:47 AM, Connor Abbott wrote:
> On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  wrote:
> >
> > Can you provide some context for this?  Those rules are already flagged 
> > "inexact" (that's what the ~ means) so they won't apply to anything that's 
> > "precise" or "invariant".
> 
> I think the concern is that this isn't allowed in SPIR-V, even without
> exact or invariant. We even go out of our way to do the correct thing
> in the frontend by inserting an "&& a == a" or "|| a != a", but then
> opt_algebraic removes it with another rule and then this rule can flip
> it from ordered to unordered. The spec says that operations don't have
> to produce NaN, but it doesn't say anything on comparisons other than
> the generic "everything must follow IEEE rules" and an entry in the
> table that says "produces correct results." Then again, I can't find
> anything in GLSL allowing these transforms either, so maybe we just
> need to get rid of them.

FYI here are the shader-db results (SKL) from removing them:

total instructions in shared programs: 12858124 -> 12889104 (0.24%)
instructions in affected programs: 1687380 -> 1718360 (1.84%)
helped: 2
HURT: 7073

total cycles in shared programs: 317838109 -> 318266406 (0.13%)
cycles in affected programs: 62285268 -> 62713565 (0.69%)
helped: 1017
HURT: 6552

total loops in shared programs: 3808 -> 3808 (0.00%)
loops in affected programs: 0 -> 0
helped: 0
HURT: 0

total spills in shared programs: 6793 -> 6785 (-0.12%)
spills in affected programs: 166 -> 158 (-4.82%)
helped: 2
HURT: 0

total fills in shared programs: 9561 -> 9541 (-0.21%)
fills in affected programs: 852 -> 832 (-2.35%)
helped: 2
HURT: 3

LOST:   0
GAINED: 1 

> 
> >
> > On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset  
> > wrote:
> >>
> >> It's correct in GLSL because the behaviour is undefined in
> >> presence of NaNs. But this seems incorrect in Vulkan.
> >>
> >> Signed-off-by: Samuel Pitoiset 
> >> ---
> >>  src/compiler/nir/nir.h| 6 ++
> >>  src/compiler/nir/nir_opt_algebraic.py | 8 
> >>  2 files changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> index db935c8496b..4107c293962 100644
> >> --- a/src/compiler/nir/nir.h
> >> +++ b/src/compiler/nir/nir.h
> >> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
> >> /* Set if nir_lower_wpos_ytransform() should also invert 
> >> gl_PointCoord. */
> >> bool lower_wpos_pntc;
> >>
> >> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
> >> +* In presence of NaNs, this is correct in GLSL because the 
> >> behaviour is
> >> +* undefined. In Vulkan, doing these transformations is incorrect.
> >> +*/
> >> +   bool exact_float_comparisons;
> >> +
> >> /**
> >>  * Should nir_lower_io() create load_interpolated_input intrinsics?
> >>  *
> >> diff --git a/src/compiler/nir/nir_opt_algebraic.py 
> >> b/src/compiler/nir/nir_opt_algebraic.py
> >> index f2a7be0c403..3750874407b 100644
> >> --- a/src/compiler/nir/nir_opt_algebraic.py
> >> +++ b/src/compiler/nir/nir_opt_algebraic.py
> >> @@ -154,10 +154,10 @@ optimizations = [
> >> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
> >>
> >> # Comparison simplifications
> >> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
> >> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
> >> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
> >> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
> >> +   (('~inot', ('flt', a, b)), ('fge', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('fge', a, b)), ('flt', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('feq', a, b)), ('fne', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('fne', a, b)), ('feq', a, b), 
> >> '!options->exact_float_comparisons'),
> >
> >
> > The feq/fne one is actually completely safe.  fne is defined to be !feq 
> > even when NaN is considered.
> >
> > --Jasoan
> >
> >>
> >> (('inot', ('ilt', a, b)), ('ige', a, b)),
> >> (('inot', ('ult', a, b)), ('uge', a, b)),
> >> (('inot', ('ige', a, b)), ('ilt', a, b)),
> >> --
> >> 2.19.2
> >>
> >> ___
> >> 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 mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Lets talk about autotools

[Matt Turner]

On Thu, Nov 29, 2018 at 5:49 PM Alex Deucher  wrote:
>
> On Thu, Nov 29, 2018 at 8:26 PM Eric Anholt  wrote:
> >
> > Emil Velikov  writes:
> >
> > > Hi all,
> > >
> > > I can see why people may opt to not use or maintain the autotools build.
> > > Although I would kindly ask that we do not remove it just yet.
> > >
> > > In Mesa, we have different parts not used by different teams. As such
> > > we tend to remove stuff when nobody is around to maintain it anymore.
> > >
> > > That said, I'm planning to continue maintaining it and would appreciate
> > > if we keep it in-tree.
> > >
> > > As people may be concerned about bugreports and alike we can trivially
> > > add a warning (as configure is invoked) to forwards any issues to my
> > > email. Additionally (or alternatively) we can have an autotools bugzilla
> > > category with me as the default assignee.
> > >
> > > What do you guys think?
> >
> > Strongly disagree.  We shouldn't be maintaining build systems for fun,
> > we should be doing the minimum amount of build system work to get our
> > actual work done quickly and reliably.
>
> If someone has a legitimate use for it and wants to maintain it, why
> not?  How is the build system any different than a specific feature in
> the project?

Restating what I said earlier, a build system is a means, not an end.
I would be sympathetic to someone wanting to maintain a driver for old
hardware because it's an end, but this is a build system that everyone
else is happy, scratch that, excited to be rid of.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Lets talk about autotools

[Alex Deucher]

On Thu, Nov 29, 2018 at 9:19 PM Jason Ekstrand  wrote:
>
> On November 29, 2018 19:49:33 Alex Deucher  wrote:
>
> > On Thu, Nov 29, 2018 at 8:26 PM Eric Anholt  wrote:
> >>
> >> Emil Velikov  writes:
> >>
> >>> Hi all,
> >>>
> >>> I can see why people may opt to not use or maintain the autotools build.
> >>> Although I would kindly ask that we do not remove it just yet.
> >>>
> >>> In Mesa, we have different parts not used by different teams. As such
> >>> we tend to remove stuff when nobody is around to maintain it anymore.
> >>>
> >>> That said, I'm planning to continue maintaining it and would appreciate
> >>> if we keep it in-tree.
> >>>
> >>> As people may be concerned about bugreports and alike we can trivially
> >>> add a warning (as configure is invoked) to forwards any issues to my
> >>> email. Additionally (or alternatively) we can have an autotools bugzilla
> >>> category with me as the default assignee.
> >>>
> >>> What do you guys think?
> >>
> >> Strongly disagree.  We shouldn't be maintaining build systems for fun,
> >> we should be doing the minimum amount of build system work to get our
> >> actual work done quickly and reliably.
> >
> > If someone has a legitimate use for it and wants to maintain it, why
> > not?  How is the build system any different than a specific feature in
> > the project?
>
> Does it have a purpose?  I know it has a maintenance burden.  How is it
> different? As long as it exists, it is going to impose that maintenance
> burden on all of the developers in the project. As long as someone is using
> autotools to build Mesa (especially if that someone is a developer), we all
> have to keep it working.

I don't personally care one way or another on this, but it's
presumably has some purpose for him.  There are all sorts of aspects
of the project that are burdens for some and essential for others.

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

Re: [Mesa-dev] Lets talk about autotools

[Jason Ekstrand]

On November 29, 2018 19:49:33 Alex Deucher  wrote:


On Thu, Nov 29, 2018 at 8:26 PM Eric Anholt  wrote:


Emil Velikov  writes:


Hi all,

I can see why people may opt to not use or maintain the autotools build.
Although I would kindly ask that we do not remove it just yet.

In Mesa, we have different parts not used by different teams. As such
we tend to remove stuff when nobody is around to maintain it anymore.

That said, I'm planning to continue maintaining it and would appreciate
if we keep it in-tree.

As people may be concerned about bugreports and alike we can trivially
add a warning (as configure is invoked) to forwards any issues to my
email. Additionally (or alternatively) we can have an autotools bugzilla
category with me as the default assignee.

What do you guys think?


Strongly disagree.  We shouldn't be maintaining build systems for fun,
we should be doing the minimum amount of build system work to get our
actual work done quickly and reliably.


If someone has a legitimate use for it and wants to maintain it, why
not?  How is the build system any different than a specific feature in
the project?


Does it have a purpose?  I know it has a maintenance burden.  How is it 
different? As long as it exists, it is going to impose that maintenance 
burden on all of the developers in the project. As long as someone is using 
autotools to build Mesa (especially if that someone is a developer), we all 
have to keep it working.


--Jason


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

Re: [Mesa-dev] [PATCH 1/3] virgl: quadruple command buffer size

[Gurchetan Singh]

Thanks!  Gentle ping for push from committers..
On Mon, Nov 26, 2018 at 11:52 AM Erik Faye-Lund
 wrote:
>
> On Mon, 2018-11-26 at 09:54 -0800, Gurchetan Singh wrote:
> > Tested running WebGL aquarium on Nvidia host (10,000 fishes)
> >
> > This moves us from 7 fps to 9 fps.  After quadrupling, performance
> > gains diminish.
> >
> > v2: Remove change ID (Erik)
> >
> > Tested-By: Gert Wollny 
> >
>
> For the series:
>
> Reviewed-by: Erik Faye-Lund 
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Lets talk about autotools

[Alex Deucher]

On Thu, Nov 29, 2018 at 8:26 PM Eric Anholt  wrote:
>
> Emil Velikov  writes:
>
> > Hi all,
> >
> > I can see why people may opt to not use or maintain the autotools build.
> > Although I would kindly ask that we do not remove it just yet.
> >
> > In Mesa, we have different parts not used by different teams. As such
> > we tend to remove stuff when nobody is around to maintain it anymore.
> >
> > That said, I'm planning to continue maintaining it and would appreciate
> > if we keep it in-tree.
> >
> > As people may be concerned about bugreports and alike we can trivially
> > add a warning (as configure is invoked) to forwards any issues to my
> > email. Additionally (or alternatively) we can have an autotools bugzilla
> > category with me as the default assignee.
> >
> > What do you guys think?
>
> Strongly disagree.  We shouldn't be maintaining build systems for fun,
> we should be doing the minimum amount of build system work to get our
> actual work done quickly and reliably.

If someone has a legitimate use for it and wants to maintain it, why
not?  How is the build system any different than a specific feature in
the project?

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

Re: [Mesa-dev] Lets talk about autotools

[Eric Anholt]

Emil Velikov  writes:

> Hi all,
>
> I can see why people may opt to not use or maintain the autotools build.
> Although I would kindly ask that we do not remove it just yet.
>
> In Mesa, we have different parts not used by different teams. As such
> we tend to remove stuff when nobody is around to maintain it anymore.
>
> That said, I'm planning to continue maintaining it and would appreciate
> if we keep it in-tree.
>
> As people may be concerned about bugreports and alike we can trivially
> add a warning (as configure is invoked) to forwards any issues to my
> email. Additionally (or alternatively) we can have an autotools bugzilla
> category with me as the default assignee.
>
> What do you guys think?

Strongly disagree.  We shouldn't be maintaining build systems for fun,
we should be doing the minimum amount of build system work to get our
actual work done quickly and reliably.


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

Re: [Mesa-dev] [PATCH] mesa: Revert INTEL_fragment_shader_ordering support

[Eric Anholt]

Matt Turner  writes:

> This extension is not properly tested (testing for
> GL_ARB_fragment_shader_interlock is not sufficient), and since this was
> noted in review on August 28th no tests have been sent.
>
> Revert "i965: Add INTEL_fragment_shader_ordering support."
> Revert "mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering"
>
> This reverts commit 03ecec9ed2099f6e2b62994b33dc948dc731e7b8.
> This reverts commit 119435c8778dd26cb7c8bcde9f04b3982239fe60.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> Emil: I just noticed that this was never reverted from master (and it
> needs to be removed before the 18.3 release)

Acked-by: Eric Anholt 


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

Re: [Mesa-dev] [PATCH v2 1/5] mesa: Add core support for EXT_multisampled_render_to_texture2

[Kristian Høgsberg]

On Tue, Nov 6, 2018 at 3:03 PM Ilia Mirkin  wrote:
>
> On Tue, Nov 6, 2018 at 5:18 PM Kristian H. Kristensen
>  wrote:
> >
> > This also turns on EXT_multisampled_render_to_texture which is a
> > subset of EXT_multisampled_render_to_texture2, allowing only
> > COLOR_ATTACHMENT0.
>
> You also enable the texture2 variant as well, no?

Yes, I'll update the subject.

>
> >
> > Signed-off-by: Kristian H. Kristensen 
> > ---
> >  .../EXT_multisampled_render_to_texture.xml| 34 ++
> >  src/mapi/glapi/gen/Makefile.am|  1 +
> >  src/mapi/glapi/gen/gl_API.xml |  2 +
> >  src/mapi/glapi/gen/meson.build|  1 +
> >  src/mesa/drivers/common/meta.c|  2 +-
> >  src/mesa/main/extensions_table.h  |  2 +
> >  src/mesa/main/fbobject.c  | 45 +--
> >  src/mesa/main/fbobject.h  |  8 +++-
> >  src/mesa/main/mtypes.h|  2 +
> >  9 files changed, 81 insertions(+), 16 deletions(-)
> >  create mode 100644 
> > src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml
> >
> > diff --git a/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml 
> > b/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml
> > new file mode 100644
> > index 000..cf44e6976f0
> > --- /dev/null
> > +++ b/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml
> > @@ -0,0 +1,34 @@
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > + > value="0x8CAB"/>
> > + > value="0x8D56"/>
> > + > value="0x8D57"/>
> > + > value="0x8D6C"/>
>
> I don't see any logic to enable retrieving these (*_SAMPLES_EXT), so I
> think this implementation is slightly incomplete.

Will add.

> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > +
> > diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
> > index 6e0ee1e1687..40538b0ff2e 100644
> > --- a/src/mapi/glapi/gen/Makefile.am
> > +++ b/src/mapi/glapi/gen/Makefile.am
> > @@ -200,6 +200,7 @@ API_XML = \
> > EXT_external_objects_fd.xml \
> > EXT_framebuffer_object.xml \
> > EXT_gpu_shader4.xml \
> > +   EXT_multisampled_render_to_texture.xml \
> > EXT_packed_depth_stencil.xml \
> > EXT_provoking_vertex.xml \
> > EXT_separate_shader_objects.xml \
> > diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
> > index aae9a5835db..ee4d13f1f06 100644
> > --- a/src/mapi/glapi/gen/gl_API.xml
> > +++ b/src/mapi/glapi/gen/gl_API.xml
> > @@ -8166,6 +8166,8 @@
> >
> >   > xmlns:xi="http://www.w3.org/2001/XInclude"/>
> >
> > + > xmlns:xi="http://www.w3.org/2001/XInclude"/>
> > +
> >   > xmlns:xi="http://www.w3.org/2001/XInclude"/>
> >
> >  
> > diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build
> > index f494e9707b6..8cc163b2989 100644
> > --- a/src/mapi/glapi/gen/meson.build
> > +++ b/src/mapi/glapi/gen/meson.build
> > @@ -107,6 +107,7 @@ api_xml_files = files(
> >'EXT_external_objects_fd.xml',
> >'EXT_framebuffer_object.xml',
> >'EXT_gpu_shader4.xml',
> > +  'EXT_multisampled_render_to_texture.xml',
> >'EXT_packed_depth_stencil.xml',
> >'EXT_provoking_vertex.xml',
> >'EXT_separate_shader_objects.xml',
> > diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> > index 4392c4bbd88..3515e312023 100644
> > --- a/src/mesa/drivers/common/meta.c
> > +++ b/src/mesa/drivers/common/meta.c
> > @@ -127,7 +127,7 @@ _mesa_meta_framebuffer_texture_image(struct gl_context 
> > *ctx,
> > assert(att);
> >
> > _mesa_framebuffer_texture(ctx, fb, attachment, att, texObj, texTarget,
> > - level, layer, false);
> > + level, att->NumSamples, layer, false);
> >  }
> >
> >  static struct gl_shader *
> > diff --git a/src/mesa/main/extensions_table.h 
> > b/src/mesa/main/extensions_table.h
> > index a516a1b17f8..f13b8b6a21a 100644
> > --- a/src/mesa/main/extensions_table.h
> > +++ b/src/mesa/main/extensions_table.h
> > @@ -241,6 +241,8 @@ EXT(EXT_map_buffer_range, 
> > ARB_map_buffer_range
> >  EXT(EXT_memory_object   , EXT_memory_object
> >   , GLL, GLC,  x , ES2, 2017)
> >  EXT(EXT_memory_object_fd, EXT_memory_object_fd 
> >   , GLL, GLC,  x , ES2, 2017)
> >  EXT(EXT_multi_draw_arrays   , dummy_true   
> >   , GLL,  x , ES1, ES2, 1999)
> > +EXT(EXT_multisampled_render_to_texture  , 
> > EXT_multisampled_render_to_texture ,  x ,  x ,  x , ES2, 2016)
> > +EXT(EXT_multisampled_render_to_texture2 , 
> > EXT_multisampled_render_to_texture ,  x ,  x ,  x , ES2, 2016)
> >  EXT(EXT_occlusion_query_boolean , ARB_occlusion_query  
> >   ,  x ,  x ,  x , ES2, 2001)
> >  

Re: [Mesa-dev] [PATCH] mesa: Revert INTEL_fragment_shader_ordering support

[Jason Ekstrand]

Acked-by: Jason Ekstrand 

On Thu, Nov 29, 2018 at 5:54 PM Matt Turner  wrote:

> This extension is not properly tested (testing for
> GL_ARB_fragment_shader_interlock is not sufficient), and since this was
> noted in review on August 28th no tests have been sent.
>
> Revert "i965: Add INTEL_fragment_shader_ordering support."
> Revert "mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering"
>
> This reverts commit 03ecec9ed2099f6e2b62994b33dc948dc731e7b8.
> This reverts commit 119435c8778dd26cb7c8bcde9f04b3982239fe60.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> Emil: I just noticed that this was never reverted from master (and it
> needs to be removed before the 18.3 release)
>
>  docs/relnotes/18.3.0.html|  1 -
>  src/compiler/glsl/builtin_functions.cpp  | 17 -
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 -
>  src/compiler/glsl/glsl_parser_extras.h   |  2 --
>  src/compiler/glsl/glsl_to_nir.cpp|  6 --
>  src/compiler/glsl/ir.h   |  1 -
>  src/compiler/nir/nir_intrinsics.py   |  1 -
>  src/intel/compiler/brw_fs_nir.cpp|  1 -
>  src/mesa/drivers/dri/i965/intel_extensions.c |  1 -
>  src/mesa/main/extensions_table.h |  1 -
>  src/mesa/main/mtypes.h   |  1 -
>  11 files changed, 33 deletions(-)
>
> diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
> index 8af225a61e1..aa924391919 100644
> --- a/docs/relnotes/18.3.0.html
> +++ b/docs/relnotes/18.3.0.html
> @@ -61,7 +61,6 @@ Note: some of the new features are only available with
> certain drivers.
>  GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
>  GL_EXT_window_rectangles on radeonsi.
>  GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
> -GL_INTEL_fragment_shader_ordering on i965.
>  GL_NV_fragment_shader_interlock on i965.
>  EGL_EXT_device_base for all drivers.
>  EGL_EXT_device_drm for all drivers.
> diff --git a/src/compiler/glsl/builtin_functions.cpp
> b/src/compiler/glsl/builtin_functions.cpp
> index 5650365d1d5..b6018806865 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -525,12 +525,6 @@ supports_nv_fragment_shader_interlock(const
> _mesa_glsl_parse_state *state)
> return state->NV_fragment_shader_interlock_enable;
>  }
>
> -static bool
> -supports_intel_fragment_shader_ordering(const _mesa_glsl_parse_state
> *state)
> -{
> -   return state->INTEL_fragment_shader_ordering_enable;
> -}
> -
>  static bool
>  shader_clock(const _mesa_glsl_parse_state *state)
>  {
> @@ -1311,11 +1305,6 @@ builtin_builder::create_intrinsics()
> supports_arb_fragment_shader_interlock,
> ir_intrinsic_end_invocation_interlock), NULL);
>
> -   add_function("__intrinsic_begin_fragment_shader_ordering",
> -_invocation_interlock_intrinsic(
> -   supports_intel_fragment_shader_ordering,
> -   ir_intrinsic_begin_fragment_shader_ordering), NULL);
> -
> add_function("__intrinsic_shader_clock",
>  _shader_clock_intrinsic(shader_clock,
>  glsl_type::uvec2_type),
> @@ -3430,12 +3419,6 @@ builtin_builder::create_builtins()
> supports_nv_fragment_shader_interlock),
>  NULL);
>
> -   add_function("beginFragmentShaderOrderingINTEL",
> -_invocation_interlock(
> -   "__intrinsic_begin_fragment_shader_ordering",
> -   supports_intel_fragment_shader_ordering),
> -NULL);
> -
> add_function("anyInvocationARB",
>  _vote("__intrinsic_vote_any", vote),
>  NULL);
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 21ed34d79d0..f9178d8c107 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -730,7 +730,6 @@ static const _mesa_glsl_extension
> _mesa_glsl_supported_extensions[] = {
> EXT_AEP(EXT_texture_buffer),
> EXT_AEP(EXT_texture_cube_map_array),
> EXT(INTEL_conservative_rasterization),
> -   EXT(INTEL_fragment_shader_ordering),
> EXT(INTEL_shader_atomic_float_minmax),
> EXT(MESA_shader_integer_functions),
> EXT(NV_fragment_shader_interlock),
> diff --git a/src/compiler/glsl/glsl_parser_extras.h
> b/src/compiler/glsl/glsl_parser_extras.h
> index da8b2fa3ab5..7ceee7469d8 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -834,8 +834,6 @@ struct _mesa_glsl_parse_state {
> bool EXT_texture_cube_map_array_warn;
> bool INTEL_conservative_rasterization_enable;
> bool INTEL_conservative_rasterization_warn;
> -   bool INTEL_fragment_shader_ordering_enable;
> -   bool INTEL_fragment_shader_ordering_warn;
> bool INTEL_shader_atomic_float_minmax_enable;
> bool 

[Mesa-dev] [PATCH] mesa: Revert INTEL_fragment_shader_ordering support

[Matt Turner]

This extension is not properly tested (testing for
GL_ARB_fragment_shader_interlock is not sufficient), and since this was
noted in review on August 28th no tests have been sent.

Revert "i965: Add INTEL_fragment_shader_ordering support."
Revert "mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering"

This reverts commit 03ecec9ed2099f6e2b62994b33dc948dc731e7b8.
This reverts commit 119435c8778dd26cb7c8bcde9f04b3982239fe60.

Cc: mesa-sta...@lists.freedesktop.org
---
Emil: I just noticed that this was never reverted from master (and it
needs to be removed before the 18.3 release)

 docs/relnotes/18.3.0.html|  1 -
 src/compiler/glsl/builtin_functions.cpp  | 17 -
 src/compiler/glsl/glsl_parser_extras.cpp |  1 -
 src/compiler/glsl/glsl_parser_extras.h   |  2 --
 src/compiler/glsl/glsl_to_nir.cpp|  6 --
 src/compiler/glsl/ir.h   |  1 -
 src/compiler/nir/nir_intrinsics.py   |  1 -
 src/intel/compiler/brw_fs_nir.cpp|  1 -
 src/mesa/drivers/dri/i965/intel_extensions.c |  1 -
 src/mesa/main/extensions_table.h |  1 -
 src/mesa/main/mtypes.h   |  1 -
 11 files changed, 33 deletions(-)

diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
index 8af225a61e1..aa924391919 100644
--- a/docs/relnotes/18.3.0.html
+++ b/docs/relnotes/18.3.0.html
@@ -61,7 +61,6 @@ Note: some of the new features are only available with 
certain drivers.
 GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
 GL_EXT_window_rectangles on radeonsi.
 GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
-GL_INTEL_fragment_shader_ordering on i965.
 GL_NV_fragment_shader_interlock on i965.
 EGL_EXT_device_base for all drivers.
 EGL_EXT_device_drm for all drivers.
diff --git a/src/compiler/glsl/builtin_functions.cpp 
b/src/compiler/glsl/builtin_functions.cpp
index 5650365d1d5..b6018806865 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -525,12 +525,6 @@ supports_nv_fragment_shader_interlock(const 
_mesa_glsl_parse_state *state)
return state->NV_fragment_shader_interlock_enable;
 }
 
-static bool
-supports_intel_fragment_shader_ordering(const _mesa_glsl_parse_state *state)
-{
-   return state->INTEL_fragment_shader_ordering_enable;
-}
-
 static bool
 shader_clock(const _mesa_glsl_parse_state *state)
 {
@@ -1311,11 +1305,6 @@ builtin_builder::create_intrinsics()
supports_arb_fragment_shader_interlock,
ir_intrinsic_end_invocation_interlock), NULL);
 
-   add_function("__intrinsic_begin_fragment_shader_ordering",
-_invocation_interlock_intrinsic(
-   supports_intel_fragment_shader_ordering,
-   ir_intrinsic_begin_fragment_shader_ordering), NULL);
-
add_function("__intrinsic_shader_clock",
 _shader_clock_intrinsic(shader_clock,
 glsl_type::uvec2_type),
@@ -3430,12 +3419,6 @@ builtin_builder::create_builtins()
supports_nv_fragment_shader_interlock),
 NULL);
 
-   add_function("beginFragmentShaderOrderingINTEL",
-_invocation_interlock(
-   "__intrinsic_begin_fragment_shader_ordering",
-   supports_intel_fragment_shader_ordering),
-NULL);
-
add_function("anyInvocationARB",
 _vote("__intrinsic_vote_any", vote),
 NULL);
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 21ed34d79d0..f9178d8c107 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -730,7 +730,6 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT_AEP(EXT_texture_buffer),
EXT_AEP(EXT_texture_cube_map_array),
EXT(INTEL_conservative_rasterization),
-   EXT(INTEL_fragment_shader_ordering),
EXT(INTEL_shader_atomic_float_minmax),
EXT(MESA_shader_integer_functions),
EXT(NV_fragment_shader_interlock),
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index da8b2fa3ab5..7ceee7469d8 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -834,8 +834,6 @@ struct _mesa_glsl_parse_state {
bool EXT_texture_cube_map_array_warn;
bool INTEL_conservative_rasterization_enable;
bool INTEL_conservative_rasterization_warn;
-   bool INTEL_fragment_shader_ordering_enable;
-   bool INTEL_fragment_shader_ordering_warn;
bool INTEL_shader_atomic_float_minmax_enable;
bool INTEL_shader_atomic_float_minmax_warn;
bool MESA_shader_integer_functions_enable;
diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index 55628dd2ccd..5e70d230550 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -747,9 +747,6 @@ 

Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

[Eric Anholt]

e<#secure method=pgpmime mode=sign>
Erik Faye-Lund  writes:

> On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
>> Jordan Justen  writes:
>> 
>> > This adds the "Developer's Certificate of Origin 1.1" from the
>> > Linux
>> > kernel. It indicates that by using Signed-off-by you are certifying
>> > that your patch meets the DCO 1.1 guidelines.
>> > 
>> > It also changes Signed-off-by from being optional to being
>> > required.
>> > 
>> > Signed-off-by: Jordan Justen 
>> > Cc: Matt Turner 
>> 
>> What problem for our project is solved by signed-off-by?  Do you
>> think
>> that it has actually reduced the incidence of people submitting code
>> they don't have permission to submit in the projects where it's been
>> used?  I know the behavior in the kernel is that people submit a
>> patch,
>> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
>> just give up.  I don't think anyone stops and says "Wow, that's a
>> good
>> question.  Maybe I don't have permission to distribute this after
>> all?"
>> 
>> /me sees s-o-b as basically just a linux kernel hazing ritual
>
> I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
> reason for the s-o-b is to have a paper-trail for how a patch landed in
> the kernel; who it went through etc.

I get the *idea*, I just believe the idea fails in practice.  The S-O-B
idea came from "wouldn't it be nice if we could make everyone think
about this issue that is important to us" but what we actually got
instead is "your patch gets bounced if you don't have a s-o-b on until
you slap one on and resend."

We've already got 1-2 people to contact if there's a question about
authorship, from the committer and author fields.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/7] EGLDevice, take 2.1

[Gurchetan Singh]

Does this require libdrm 2.4.95 for virtio-gpu?  With
platform_surfaceless, I keep getting "DRI2: failed to find EGLDevice"
and the failure is happening in "drmGetDevice2(fd, 0, )".On
Sat, Oct 20, 2018 at 11:46 PM Mathias Fröhlich
 wrote:
>
> Hi Emil,
>
> On Wednesday, 3 October 2018 16:02:40 CEST Emil Velikov wrote:
> > This re-spin of the series includes:
> >  - correct flipped asserts
> >  - cosmetic wording/comment fixes
> >  - drop EGL_EXT_platform_device patches (swrast is broken)
> >  - add the EGL_MESA_device_software spec patch
> >
> > At this point we should be pretty much set, so any formal Ack/Rb will
> > be appreciated.
>
> Sure and sorry for the long delay!
>
> Patch #1-#6 is:
> Reviewed-by: Mathias Fröhlich 
>
> The #7 is already reviewed by Dylan, so the series should be complete now.
>
> Thanks for iterating on that.
> And looking forward to the rest of the patches enabling display
> creation from a device!!
>
> best
> Mathias
>
>
> >
> > Thanks
> > Emil
> >
> > Cc: Adam Jackson 
> > Cc: Eric Engestrom 
> > Cc: Mathias Fröhlich 
> >
> > Adam Jackson (1):
> >   specs: Add EGL_MESA_device_software
> >
> > Emil Velikov (6):
> >   egl: add base EGL_EXT_device_base implementation
> >   egl: add EGL_MESA_device_software support
> >   egl: add EGL_EXT_device_drm support
> >   egl: set the EGLDevice when creating a display
> >   egl: enable EGL_EXT_device_{base,enumeration,query}
> >   meson: egl: group dri2 bits separately from haiku
> >
> >  docs/specs/EGL_MESA_device_software.txt |  82 +
> >  src/egl/Makefile.sources|   2 +
> >  src/egl/drivers/dri2/egl_dri2.h |   1 +
> >  src/egl/drivers/dri2/platform_android.c |   9 +
> >  src/egl/drivers/dri2/platform_drm.c |   9 +
> >  src/egl/drivers/dri2/platform_surfaceless.c |  10 +-
> >  src/egl/drivers/dri2/platform_wayland.c |  18 ++
> >  src/egl/drivers/dri2/platform_x11.c |  27 ++
> >  src/egl/drivers/haiku/egl_haiku.cpp |   8 +
> >  src/egl/main/eglapi.c   |  64 
> >  src/egl/main/egldevice.c| 319 
> >  src/egl/main/egldevice.h|  89 ++
> >  src/egl/main/egldisplay.h   |   1 +
> >  src/egl/main/eglentrypoint.h|   4 +
> >  src/egl/main/eglglobals.c   |  12 +-
> >  src/egl/main/eglglobals.h   |   2 +
> >  src/egl/main/egltypedefs.h  |   2 +
> >  src/egl/meson.build |  73 ++---
> >  18 files changed, 692 insertions(+), 40 deletions(-)
> >  create mode 100644 docs/specs/EGL_MESA_device_software.txt
> >  create mode 100644 src/egl/main/egldevice.c
> >  create mode 100644 src/egl/main/egldevice.h
> >
> >
>
>
>
>
> ___
> 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] docs: Document optional GitLab code review process

[Eric Anholt]

Eric Engestrom  writes:

> On Wednesday, 2018-11-28 13:36:29 -0800, Eric Anholt wrote:
>> Jordan Justen  writes:
>> 
>> > This documents a mechanism for using GitLab Merge Requests as an
>> > optional, secondary way to get code reviews for patchsets.
>> >
>> > We still require all patches to be emailed.
>> >
>> > Aside from the potential usage for code review comments, it might also
>> > help reviewers to find unmerged patchsets which need review. (Assuming
>> > it doesn't fall into the same fate of patchwork with hundreds of open
>> > MRs.)
>> >
>> > Signed-off-by: Jordan Justen 
>> > Cc: Jason Ekstrand 
>> > ---
>> >  docs/submittingpatches.html | 25 +
>> >  1 file changed, 25 insertions(+)
>> >
>> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
>> > index 5d8ba443191..852f28c198a 100644
>> > --- a/docs/submittingpatches.html
>> > +++ b/docs/submittingpatches.html
>> > @@ -24,6 +24,7 @@
>> >  Testing Patches
>> >  Mailing Patches
>> >  Reviewing Patches
>> > +GitLab Merge Requests
>> >  Nominating a commit for a stable branch
>> >  Criteria for accepting patches to the stable 
>> > branch
>> >  Sending backports for the stable branch
>> > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
>> > committed, as long
>> >  as the issues are resolved first.
>> >  
>> >  
>> > +GitLab Merge Requests
>> > +
>> > +
>> > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
>> > +  Requests (MR) can be used as an optional, secondary method of
>> > +  obtaining code review for patches.
>> > +
>> > +
>> > +
>> > +  All patches should be submitted using email as well
>> 
>> Like others, I disagree and think this will lead to confusion.  Let
>> people send to one or the other.
>> 
>> > +  Consider including a link to the MR in your email based
>> > +cover-letter
>> > +  Address code review from both email and the MR
>> > +  Add appropriate labels to your MR. For example:
>> > +
>> > +  Mesa changes affect all drivers: mesa
>> > +  Hardware vendor specific code: amd, intel, nvidia, etc
>> > +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
>> > radv, vc4, etc
>> > +  Other tag examples: gallium, util
>> > +
>> > +  Never use the merge button on the GitLab page to push patches
>> 
>> Why "never use the merge button"?  We've been using rebase+merge in
>> xserver and it's *awesome* and has greatly increased the review rate.
>
> Because GitLab doesn't (yet) support tracking 'reviewed' status in
> the commit message, which is something I think is important to track for
> various reasons. If those are manually added to the commit messages
> before the MR is merged, then there's no issue there.
>
> We could also have a check that an MR can't be merged until all its
> commits contain the string "Reviewed-by:", which might be a better
> solution?

In the xserver we decided that it was more important to have MRs be
available (so a reviewer can click "merge when the tests are green")
than to have in-git tracking of whether commits were reviewed.  I think
it's been a great tradeoff, but I understand if Mesa doesn't want to
make the same one.


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

[Mesa-dev] [PATCH v2 03/11] nir/algebraic: Clean up some __str__ cruft

[Jason Ekstrand]

Both of these things are already handled in the Value base class so we
don't need to handle them explicitly in Constant.
---
 src/compiler/nir/nir_algebraic.py | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 3938cdcfc06..b19a53637b7 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -185,7 +185,6 @@ class Constant(Value):
def __init__(self, val, name):
   Value.__init__(self, val, name, "constant")
 
-  self.in_val = str(val)
   if isinstance(val, (str)):
  m = _constant_re.match(val)
  self.value = ast.literal_eval(m.group('value'))
@@ -251,9 +250,6 @@ class Variable(Value):
 
   self.index = varset[self.var_name]
 
-   def __str__(self):
-  return self.in_val
-
def type(self):
   if self.required_type == 'bool':
  return "nir_type_bool"
-- 
2.19.1

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

[Mesa-dev] [PATCH v2 07/11] nir/opt_algebraic: Drop bit-size suffixes from conversions

[Jason Ekstrand]

Suffixes are dropped from a bunch of conversion opcodes when it makes
sense to do so.  Others are kept if we really do want the bit-size
restriction.
---
 src/compiler/nir/nir_opt_algebraic.py | 58 +--
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/compiler/nir/nir_opt_algebraic.py 
b/src/compiler/nir/nir_opt_algebraic.py
index 27c90cebaee..c482bde8b3b 100644
--- a/src/compiler/nir/nir_opt_algebraic.py
+++ b/src/compiler/nir/nir_opt_algebraic.py
@@ -82,7 +82,7 @@ optimizations = [
(('ineg', ('ineg', a)), a),
(('fabs', ('fabs', a)), ('fabs', a)),
(('fabs', ('fneg', a)), ('fabs', a)),
-   (('fabs', ('u2f32', a)), ('u2f32', a)),
+   (('fabs', ('u2f', a)), ('u2f', a)),
(('iabs', ('iabs', a)), ('iabs', a)),
(('iabs', ('ineg', a)), ('iabs', a)),
(('~fadd', a, 0.0), a),
@@ -242,18 +242,18 @@ optimizations = [
(('ieq', ('b2i', a), 0),   ('inot', a)),
(('ine', ('b2i', a), 0),   a),
 
-   (('fne', ('u2f32', a), 0.0), ('ine', a, 0)),
-   (('feq', ('u2f32', a), 0.0), ('ieq', a, 0)),
-   (('fge', ('u2f32', a), 0.0), True),
-   (('fge', 0.0, ('u2f32', a)), ('uge', 0, a)),# ieq instead?
-   (('flt', ('u2f32', a), 0.0), False),
-   (('flt', 0.0, ('u2f32', a)), ('ult', 0, a)),# ine instead?
-   (('fne', ('i2f32', a), 0.0), ('ine', a, 0)),
-   (('feq', ('i2f32', a), 0.0), ('ieq', a, 0)),
-   (('fge', ('i2f32', a), 0.0), ('ige', a, 0)),
-   (('fge', 0.0, ('i2f32', a)), ('ige', 0, a)),
-   (('flt', ('i2f32', a), 0.0), ('ilt', a, 0)),
-   (('flt', 0.0, ('i2f32', a)), ('ilt', 0, a)),
+   (('fne', ('u2f', a), 0.0), ('ine', a, 0)),
+   (('feq', ('u2f', a), 0.0), ('ieq', a, 0)),
+   (('fge', ('u2f', a), 0.0), True),
+   (('fge', 0.0, ('u2f', a)), ('uge', 0, a)),# ieq instead?
+   (('flt', ('u2f', a), 0.0), False),
+   (('flt', 0.0, ('u2f', a)), ('ult', 0, a)),# ine instead?
+   (('fne', ('i2f', a), 0.0), ('ine', a, 0)),
+   (('feq', ('i2f', a), 0.0), ('ieq', a, 0)),
+   (('fge', ('i2f', a), 0.0), ('ige', a, 0)),
+   (('fge', 0.0, ('i2f', a)), ('ige', 0, a)),
+   (('flt', ('i2f', a), 0.0), ('ilt', a, 0)),
+   (('flt', 0.0, ('i2f', a)), ('ilt', 0, a)),
 
# 0.0 < fabs(a)
# fabs(a) > 0.0
@@ -534,8 +534,8 @@ optimizations = [
# Conversions
(('i2b', ('b2i', a)), a),
(('i2b', 'a@bool'), a),
-   (('f2i32', ('ftrunc', a)), ('f2i32', a)),
-   (('f2u32', ('ftrunc', a)), ('f2u32', a)),
+   (('f2i', ('ftrunc', a)), ('f2i', a)),
+   (('f2u', ('ftrunc', a)), ('f2u', a)),
(('i2b', ('ineg', a)), ('i2b', a)),
(('i2b', ('iabs', a)), ('i2b', a)),
(('fabs', ('b2f', a)), ('b2f', a)),
@@ -545,10 +545,10 @@ optimizations = [
# Ironically, mark these as imprecise because removing the conversions may
# preserve more precision than doing the conversions (e.g.,
# uint(float(0x81818181u)) == 0x81818200).
-   (('~f2i32', ('i2f32', 'a@32')), a),
-   (('~f2i32', ('u2f32', 'a@32')), a),
-   (('~f2u32', ('i2f32', 'a@32')), a),
-   (('~f2u32', ('u2f32', 'a@32')), a),
+   (('~f2i32', ('i2f', 'a@32')), a),
+   (('~f2i32', ('u2f', 'a@32')), a),
+   (('~f2u32', ('i2f', 'a@32')), a),
+   (('~f2u32', ('u2f', 'a@32')), a),
 
# Packing and then unpacking does nothing
(('unpack_64_2x32_split_x', ('pack_64_2x32_split', a, b)), a),
@@ -718,29 +718,29 @@ optimizations = [
 
 (('unpack_unorm_2x16', 'v'),
  ('fdiv', ('u2f32', ('vec2', ('extract_u16', 'v', 0),
- ('extract_u16', 'v', 1))),
+  ('extract_u16', 'v', 1))),
   65535.0),
  'options->lower_unpack_unorm_2x16'),
 
 (('unpack_unorm_4x8', 'v'),
  ('fdiv', ('u2f32', ('vec4', ('extract_u8', 'v', 0),
- ('extract_u8', 'v', 1),
- ('extract_u8', 'v', 2),
- ('extract_u8', 'v', 3))),
+  ('extract_u8', 'v', 1),
+  ('extract_u8', 'v', 2),
+  ('extract_u8', 'v', 3))),
   255.0),
  'options->lower_unpack_unorm_4x8'),
 
 (('unpack_snorm_2x16', 'v'),
- ('fmin', 1.0, ('fmax', -1.0, ('fdiv', ('i2f32', ('vec2', ('extract_i16', 
'v', 0),
-  ('extract_i16', 
'v', 1))),
+ ('fmin', 1.0, ('fmax', -1.0, ('fdiv', ('i2f', ('vec2', ('extract_i16', 
'v', 0),
+('extract_i16', 
'v', 1))),
32767.0))),
  'options->lower_unpack_snorm_2x16'),
 
 (('unpack_snorm_4x8', 'v'),
- ('fmin', 1.0, ('fmax', -1.0, ('fdiv', ('i2f32', ('vec4', ('extract_i8', 
'v', 0),
-  ('extract_i8', 
'v', 1),
-  ('extract_i8', 
'v', 2),
-  ('extract_i8', 
'v', 3))),
+ ('fmin', 

[Mesa-dev] [PATCH v2 04/11] nir/algebraic: Refactor codegen a bit

[Jason Ekstrand]

Instead of using an OrderedDict, just have a (necessarily sorted) array
of transforms and a set of opcodes.
---
 src/compiler/nir/nir_algebraic.py | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index b19a53637b7..8b137f2a021 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -25,7 +25,7 @@
 
 from __future__ import print_function
 import ast
-from collections import OrderedDict
+from collections import defaultdict
 import itertools
 import struct
 import sys
@@ -610,12 +610,12 @@ struct transform {
 
 #endif
 
-% for (opcode, xform_list) in xform_dict.items():
-% for xform in xform_list:
+% for xform in xforms:
${xform.search.render()}
${xform.replace.render()}
 % endfor
 
+% for (opcode, xform_list) in sorted(opcode_xforms.items()):
 static const struct transform ${pass_name}_${opcode}_xforms[] = {
 % for xform in xform_list:
{ &${xform.search.name}, ${xform.replace.c_ptr}, ${xform.condition_index} },
@@ -638,7 +638,7 @@ ${pass_name}_block(nir_builder *build, nir_block *block,
  continue;
 
   switch (alu->op) {
-  % for opcode in xform_dict.keys():
+  % for opcode in sorted(opcode_xforms.keys()):
   case nir_op_${opcode}:
  for (unsigned i = 0; i < ARRAY_SIZE(${pass_name}_${opcode}_xforms); 
i++) {
 const struct transform *xform = &${pass_name}_${opcode}_xforms[i];
@@ -701,7 +701,8 @@ ${pass_name}(nir_shader *shader)
 
 class AlgebraicPass(object):
def __init__(self, pass_name, transforms):
-  self.xform_dict = OrderedDict()
+  self.xforms = []
+  self.opcode_xforms = defaultdict(lambda : [])
   self.pass_name = pass_name
 
   error = False
@@ -718,15 +719,15 @@ class AlgebraicPass(object):
error = True
continue
 
- if xform.search.opcode not in self.xform_dict:
-self.xform_dict[xform.search.opcode] = []
-
- self.xform_dict[xform.search.opcode].append(xform)
+ self.xforms.append(xform)
+ self.opcode_xforms[xform.search.opcode].append(xform)
 
   if error:
  sys.exit(1)
 
+
def render(self):
   return _algebraic_pass_template.render(pass_name=self.pass_name,
- xform_dict=self.xform_dict,
+ xforms=self.xforms,
+ opcode_xforms=self.opcode_xforms,
  condition_list=condition_list)
-- 
2.19.1

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

[Mesa-dev] [PATCH v2 09/11] nir: Make boolean conversions sized just like the others

[Jason Ekstrand]

Instead of a single i2b and b2i, we now have i2b32 and b2iN where N is
one if 8, 16, 32, or 64.  This leads to having a few more opcodes but
now everything is consistent and booleans aren't a weird special case
anymore.
---
 src/compiler/nir/nir.h |  4 ++--
 src/compiler/nir/nir_algebraic.py  |  4 
 src/compiler/nir/nir_lower_idiv.c  |  2 +-
 src/compiler/nir/nir_lower_int64.c |  2 +-
 src/compiler/nir/nir_opcodes.py| 25 +
 src/compiler/nir/nir_opcodes_c.py  | 30 --
 src/compiler/nir/nir_opt_if.c  |  2 +-
 src/compiler/nir/nir_search.c  | 19 +++
 src/compiler/nir/nir_search.h  |  4 
 9 files changed, 57 insertions(+), 35 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index db935c8496b..cb8122def8a 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -1568,8 +1568,8 @@ nir_alu_instr_is_comparison(const nir_alu_instr *instr)
case nir_op_uge:
case nir_op_ieq:
case nir_op_ine:
-   case nir_op_i2b:
-   case nir_op_f2b:
+   case nir_op_i2b32:
+   case nir_op_f2b32:
case nir_op_inot:
case nir_op_fnot:
   return true;
diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index def0f94da48..ce77db80a40 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -45,6 +45,10 @@ conv_opcode_types = {
 'f2i' : 'int',
 'u2u' : 'uint',
 'i2i' : 'int',
+'b2f' : 'float',
+'b2i' : 'int',
+'i2b' : 'bool',
+'f2b' : 'bool',
 }
 
 if sys.version_info < (3, 0):
diff --git a/src/compiler/nir/nir_lower_idiv.c 
b/src/compiler/nir/nir_lower_idiv.c
index 194ca5a75a8..b2a0a3c1899 100644
--- a/src/compiler/nir/nir_lower_idiv.c
+++ b/src/compiler/nir/nir_lower_idiv.c
@@ -95,7 +95,7 @@ convert_instr(nir_builder *bld, nir_alu_instr *alu)
r = nir_isub(bld, a, r);
 
r = nir_uge(bld, r, b);
-   r = nir_b2i(bld, r);
+   r = nir_b2i32(bld, r);
 
q = nir_iadd(bld, q, r);
if (is_signed)  {
diff --git a/src/compiler/nir/nir_lower_int64.c 
b/src/compiler/nir/nir_lower_int64.c
index 50acc858605..81669c02cc6 100644
--- a/src/compiler/nir/nir_lower_int64.c
+++ b/src/compiler/nir/nir_lower_int64.c
@@ -48,7 +48,7 @@ lower_isign64(nir_builder *b, nir_ssa_def *x)
 
nir_ssa_def *is_non_zero = nir_i2b(b, nir_ior(b, x_lo, x_hi));
nir_ssa_def *res_hi = nir_ishr(b, x_hi, nir_imm_int(b, 31));
-   nir_ssa_def *res_lo = nir_ior(b, res_hi, nir_b2i(b, is_non_zero));
+   nir_ssa_def *res_lo = nir_ior(b, res_hi, nir_b2i32(b, is_non_zero));
 
return nir_pack_64_2x32_split(b, res_lo, res_hi);
 }
diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index 00720708305..55b2ff0f678 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -90,6 +90,7 @@ class Opcode(object):
 # helper variables for strings
 tfloat = "float"
 tint = "int"
+tbool = "bool"
 tbool32 = "bool32"
 tuint = "uint"
 tuint16 = "uint16"
@@ -117,6 +118,8 @@ def type_size(type_):
 def type_sizes(type_):
 if type_has_size(type_):
 return [type_size(type_)]
+elif type_ == 'bool':
+return [32]
 elif type_ == 'float':
 return [16, 32, 64]
 else:
@@ -196,11 +199,15 @@ unop("fexp2", tfloat, "exp2f(src0)")
 unop("flog2", tfloat, "log2f(src0)")
 
 # Generate all of the numeric conversion opcodes
-for src_t in [tint, tuint, tfloat]:
-   if src_t in (tint, tuint):
-  dst_types = [tfloat, src_t]
+for src_t in [tint, tuint, tfloat, tbool]:
+   if src_t == tbool:
+  dst_types = [tfloat, tint]
+   elif src_t == tint:
+  dst_types = [tfloat, tint, tbool]
+   elif src_t == tuint:
+  dst_types = [tfloat, tuint]
elif src_t == tfloat:
-  dst_types = [tint, tuint, tfloat]
+  dst_types = [tint, tuint, tfloat, tbool]
 
for dst_t in dst_types:
   for bit_size in type_sizes(dst_t):
@@ -211,15 +218,9 @@ for src_t in [tint, tuint, tfloat]:
bit_size, rnd_mode),
dst_t + str(bit_size), src_t, "src0")
   else:
+  conv_expr = "src0 != 0" if dst_t == tbool else "src0"
   unop_convert("{0}2{1}{2}".format(src_t[0], dst_t[0], bit_size),
-   dst_t + str(bit_size), src_t, "src0")
-
-# We'll hand-code the to/from bool conversion opcodes.  Because bool doesn't
-# have multiple bit-sizes, we can always infer the size from the other type.
-unop_convert("f2b", tbool32, tfloat, "src0 != 0.0")
-unop_convert("i2b", tbool32, tint, "src0 != 0")
-unop_convert("b2f", tfloat, tbool32, "src0 ? 1.0 : 0.0")
-unop_convert("b2i", tint, tbool32, "src0 ? 1 : 0")
+   dst_t + str(bit_size), src_t, conv_expr)
 
 
 # Unary floating-point rounding operations.
diff --git a/src/compiler/nir/nir_opcodes_c.py 
b/src/compiler/nir/nir_opcodes_c.py
index 9e3c06b8634..017c8b7ea9a 100644
--- 

[Mesa-dev] [PATCH v2 01/11] nir/opcodes: Pull in the type helpers from constant_expressions

[Jason Ekstrand]

While we're at it, we rework them a bit to all use regular expressions
and assert more.
---
 src/compiler/nir/nir_constant_expressions.py | 25 ++
 src/compiler/nir/nir_opcodes.py  | 34 +---
 src/compiler/nir/nir_opcodes_c.py| 11 ++-
 3 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/src/compiler/nir/nir_constant_expressions.py 
b/src/compiler/nir/nir_constant_expressions.py
index 118af9f7818..0cd4ffcf558 100644
--- a/src/compiler/nir/nir_constant_expressions.py
+++ b/src/compiler/nir/nir_constant_expressions.py
@@ -1,23 +1,8 @@
 from __future__ import print_function
 
 import re
-
-type_split_re = re.compile(r'(?P[a-z]+)(?P\d+)')
-
-def type_has_size(type_):
-return type_[-1:].isdigit()
-
-def type_size(type_):
-assert type_has_size(type_)
-return int(type_split_re.match(type_).group('bits'))
-
-def type_sizes(type_):
-if type_has_size(type_):
-return [type_size(type_)]
-elif type_ == 'float':
-return [16, 32, 64]
-else:
-return [8, 16, 32, 64]
+from nir_opcodes import opcodes
+from nir_opcodes import type_has_size, type_size, type_sizes, type_base_type
 
 def type_add_size(type_, size):
 if type_has_size(type_):
@@ -44,10 +29,7 @@ def get_const_field(type_):
 elif type_ == "float16":
 return "u16"
 else:
-m = type_split_re.match(type_)
-if not m:
-raise Exception(str(type_))
-return m.group('type')[0] + m.group('bits')
+return type_base_type(type_)[0] + str(type_size(type_))
 
 template = """\
 /*
@@ -429,7 +411,6 @@ nir_eval_const_opcode(nir_op op, unsigned num_components,
}
 }"""
 
-from nir_opcodes import opcodes
 from mako.template import Template
 
 print(Template(template).render(opcodes=opcodes, type_sizes=type_sizes,
diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index 4ef4ecc6f22..d69d09d30ce 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -23,6 +23,7 @@
 # Authors:
 #Connor Abbott (cwabbo...@gmail.com)
 
+import re
 
 # Class that represents all the information we have about the opcode
 # NOTE: this must be kept in sync with nir_op_info
@@ -99,6 +100,33 @@ tint64 = "int64"
 tuint64 = "uint64"
 tfloat64 = "float64"
 
+_TYPE_SPLIT_RE = re.compile(r'(?Pint|uint|float|bool)(?P\d+)?')
+
+def type_has_size(type_):
+m = _TYPE_SPLIT_RE.match(type_)
+assert m is not None, 'Invalid NIR type string: "{}"'.format(type_)
+return m.group('bits') is not None
+
+def type_size(type_):
+m = _TYPE_SPLIT_RE.match(type_)
+assert m is not None, 'Invalid NIR type string: "{}"'.format(type_)
+assert m.group('bits') is not None, \
+   'NIR type string has no bit size: "{}"'.format(type_)
+return int(m.group('bits'))
+
+def type_sizes(type_):
+if type_has_size(type_):
+return [type_size(type_)]
+elif type_ == 'float':
+return [16, 32, 64]
+else:
+return [8, 16, 32, 64]
+
+def type_base_type(type_):
+m = _TYPE_SPLIT_RE.match(type_)
+assert m is not None, 'Invalid NIR type string: "{}"'.format(type_)
+return m.group('type')
+
 commutative = "commutative "
 associative = "associative "
 
@@ -175,11 +203,7 @@ for src_t in [tint, tuint, tfloat]:
   dst_types = [tint, tuint, tfloat]
 
for dst_t in dst_types:
-  if dst_t == tfloat:
- bit_sizes = [16, 32, 64]
-  else:
- bit_sizes = [8, 16, 32, 64]
-  for bit_size in bit_sizes:
+  for bit_size in type_sizes(dst_t):
   if bit_size == 16 and dst_t == tfloat and src_t == tfloat:
   rnd_modes = ['_rtne', '_rtz', '']
   for rnd_mode in rnd_modes:
diff --git a/src/compiler/nir/nir_opcodes_c.py 
b/src/compiler/nir/nir_opcodes_c.py
index 8bfcda6d719..9e3c06b8634 100644
--- a/src/compiler/nir/nir_opcodes_c.py
+++ b/src/compiler/nir/nir_opcodes_c.py
@@ -25,7 +25,7 @@
 
 from __future__ import print_function
 
-from nir_opcodes import opcodes
+from nir_opcodes import opcodes, type_sizes
 from mako.template import Template
 
 template = Template("""
@@ -64,12 +64,7 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type dst, 
nir_rounding_mode rnd
 % endif
 %  endif
switch (dst_bit_size) {
-% if dst_t == 'float':
-<%bit_sizes = [16, 32, 64] %>
-% else:
-<%bit_sizes = [8, 16, 32, 64] %>
-% endif
-% for dst_bits in bit_sizes:
+% for dst_bits in type_sizes(dst_t):
   case ${dst_bits}:
 %if src_t == 'float' and dst_t == 'float' and dst_bits == 
16:
  switch(rnd) {
@@ -137,4 +132,4 @@ const nir_op_info nir_op_infos[nir_num_opcodes] = {
 };
 """)
 
-print(template.render(opcodes=opcodes))
+print(template.render(opcodes=opcodes, type_sizes=type_sizes))
-- 
2.19.1

[Mesa-dev] [PATCH v2 08/11] nir/opt_algebraic: Add 32-bit specifiers to a bunch of booleans

[Jason Ekstrand]

---
 src/compiler/nir/nir_opt_algebraic.py | 112 +-
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/src/compiler/nir/nir_opt_algebraic.py 
b/src/compiler/nir/nir_opt_algebraic.py
index c482bde8b3b..aa1a7a94e6e 100644
--- a/src/compiler/nir/nir_opt_algebraic.py
+++ b/src/compiler/nir/nir_opt_algebraic.py
@@ -119,17 +119,17 @@ optimizations = [
(('~flrp', a, b, 1.0), b),
(('~flrp', a, a, b), a),
(('~flrp', 0.0, a, b), ('fmul', a, b)),
-   (('~flrp', a, b, ('b2f', c)), ('bcsel', c, b, a), 'options->lower_flrp32'),
+   (('~flrp', a, b, ('b2f', 'c@32')), ('bcsel', c, b, a), 
'options->lower_flrp32'),
(('~flrp', a, 0.0, c), ('fadd', ('fmul', ('fneg', a), c), a)),
(('flrp@32', a, b, c), ('fadd', ('fmul', c, ('fsub', b, a)), a), 
'options->lower_flrp32'),
(('flrp@64', a, b, c), ('fadd', ('fmul', c, ('fsub', b, a)), a), 
'options->lower_flrp64'),
(('ffloor', a), ('fsub', a, ('ffract', a)), 'options->lower_ffloor'),
(('ffract', a), ('fsub', a, ('ffloor', a)), 'options->lower_ffract'),
(('fceil', a), ('fneg', ('ffloor', ('fneg', a))), 'options->lower_fceil'),
-   (('~fadd', ('fmul', a, ('fadd', 1.0, ('fneg', ('b2f', c, ('fmul', b, 
('b2f', c))), ('bcsel', c, b, a), 'options->lower_flrp32'),
+   (('~fadd', ('fmul', a, ('fadd', 1.0, ('fneg', ('b2f', 'c@32', ('fmul', 
b, ('b2f', c))), ('bcsel', c, b, a), 'options->lower_flrp32'),
(('~fadd@32', ('fmul', a, ('fadd', 1.0, ('fneg', c ))), ('fmul', b, 
c )), ('flrp', a, b, c), '!options->lower_flrp32'),
(('~fadd@64', ('fmul', a, ('fadd', 1.0, ('fneg', c ))), ('fmul', b, 
c )), ('flrp', a, b, c), '!options->lower_flrp64'),
-   (('~fadd', a, ('fmul', ('b2f', c), ('fadd', b, ('fneg', a, ('bcsel', c, 
b, a), 'options->lower_flrp32'),
+   (('~fadd', a, ('fmul', ('b2f', 'c@32'), ('fadd', b, ('fneg', a, 
('bcsel', c, b, a), 'options->lower_flrp32'),
(('~fadd@32', a, ('fmul', c , ('fadd', b, ('fneg', a, ('flrp', 
a, b, c), '!options->lower_flrp32'),
(('~fadd@64', a, ('fmul', c , ('fadd', b, ('fneg', a, ('flrp', 
a, b, c), '!options->lower_flrp64'),
(('ffma', a, b, c), ('fadd', ('fmul', a, b), c), 'options->lower_ffma'),
@@ -169,50 +169,50 @@ optimizations = [
# b2f(a) <= 0.0
# b2f(a) == 0.0 because b2f(a) can only be 0 or 1
# inot(a)
-   (('fge', 0.0, ('b2f', a)), ('inot', a)),
-
-   (('fge', ('fneg', ('b2f', a)), 0.0), ('inot', a)),
-
-   (('fne', ('fadd', ('b2f', a), ('b2f', b)), 0.0), ('ior', a, b)),
-   (('fne', ('fmax', ('b2f', a), ('b2f', b)), 0.0), ('ior', a, b)),
-   (('fne', ('bcsel', a, 1.0, ('b2f', b))   , 0.0), ('ior', a, b)),
-   (('fne', ('b2f', a), ('fneg', ('b2f', b))),  ('ior', a, b)),
-   (('fne', ('fmul', ('b2f', a), ('b2f', b)), 0.0), ('iand', a, b)),
-   (('fne', ('fmin', ('b2f', a), ('b2f', b)), 0.0), ('iand', a, b)),
-   (('fne', ('bcsel', a, ('b2f', b), 0.0)   , 0.0), ('iand', a, b)),
-   (('fne', ('fadd', ('b2f', a), ('fneg', ('b2f', b))), 0.0), ('ixor', a, b)),
-   (('fne',  ('b2f', a) ,  ('b2f', b) ),  ('ixor', a, b)),
-   (('fne', ('fneg', ('b2f', a)), ('fneg', ('b2f', b))),  ('ixor', a, b)),
-   (('feq', ('fadd', ('b2f', a), ('b2f', b)), 0.0), ('inot', ('ior', a, b))),
-   (('feq', ('fmax', ('b2f', a), ('b2f', b)), 0.0), ('inot', ('ior', a, b))),
-   (('feq', ('bcsel', a, 1.0, ('b2f', b))   , 0.0), ('inot', ('ior', a, b))),
-   (('feq', ('b2f', a), ('fneg', ('b2f', b))),  ('inot', ('ior', a, b))),
-   (('feq', ('fmul', ('b2f', a), ('b2f', b)), 0.0), ('inot', ('iand', a, b))),
-   (('feq', ('fmin', ('b2f', a), ('b2f', b)), 0.0), ('inot', ('iand', a, b))),
-   (('feq', ('bcsel', a, ('b2f', b), 0.0)   , 0.0), ('inot', ('iand', a, b))),
-   (('feq', ('fadd', ('b2f', a), ('fneg', ('b2f', b))), 0.0), ('ieq', a, b)),
-   (('feq',  ('b2f', a) ,  ('b2f', b) ),  ('ieq', a, b)),
-   (('feq', ('fneg', ('b2f', a)), ('fneg', ('b2f', b))),  ('ieq', a, b)),
+   (('fge', 0.0, ('b2f', 'a@32')), ('inot', a)),
+
+   (('fge', ('fneg', ('b2f', 'a@32')), 0.0), ('inot', a)),
+
+   (('fne', ('fadd', ('b2f', 'a@32'), ('b2f', 'b@32')), 0.0), ('ior', a, b)),
+   (('fne', ('fmax', ('b2f', 'a@32'), ('b2f', 'b@32')), 0.0), ('ior', a, b)),
+   (('fne', ('bcsel', a, 1.0, ('b2f', 'b@32'))   , 0.0), ('ior', a, b)),
+   (('fne', ('b2f', 'a@32'), ('fneg', ('b2f', 'b@32'))),  ('ior', a, b)),
+   (('fne', ('fmul', ('b2f', 'a@32'), ('b2f', 'b@32')), 0.0), ('iand', a, b)),
+   (('fne', ('fmin', ('b2f', 'a@32'), ('b2f', 'b@32')), 0.0), ('iand', a, b)),
+   (('fne', ('bcsel', a, ('b2f', 'b@32'), 0.0)   , 0.0), ('iand', a, b)),
+   (('fne', ('fadd', ('b2f', 'a@32'), ('fneg', ('b2f', 'b@32'))), 0.0), 
('ixor', a, b)),
+   (('fne',  ('b2f', 'a@32') ,  ('b2f', 'b@32') ),  
('ixor', a, b)),
+   (('fne', ('fneg', ('b2f', 'a@32')), ('fneg', ('b2f', 'b@32'))),  
('ixor', a, b)),
+   (('feq', ('fadd', ('b2f', 'a@32'), ('b2f', 'b@32')), 0.0), 

[Mesa-dev] [PATCH v2 06/11] nir/opt_algebraic: Simplify an optimization using the new search ops

[Jason Ekstrand]

---
 src/compiler/nir/nir_opt_algebraic.py | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/compiler/nir/nir_opt_algebraic.py 
b/src/compiler/nir/nir_opt_algebraic.py
index f2a7be0c403..27c90cebaee 100644
--- a/src/compiler/nir/nir_opt_algebraic.py
+++ b/src/compiler/nir/nir_opt_algebraic.py
@@ -762,13 +762,8 @@ for x, y in itertools.product(['f', 'u', 'i'], ['f', 'u', 
'i']):
 
b2x = 'b2f' if x == 'f' else 'b2i'
b2y = 'b2f' if y == 'f' else 'b2i'
-
-   for N in [8, 16, 32, 64]:
-  if y == 'f' and N == 8:
- continue
-
-  x2yN = '{}2{}{}'.format(x, y, N)
-  optimizations.append(((x2yN, (b2x, a)), (b2y, a)))
+   x2yN = '{}2{}'.format(x, y)
+   optimizations.append(((x2yN, (b2x, a)), (b2y, a)))
 
 def fexp2i(exp, bits):
# We assume that exp is already in the right range.
-- 
2.19.1

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

[Mesa-dev] [PATCH v2 02/11] nir/opcodes: Rename tbool to tbool32

[Jason Ekstrand]

---
 src/compiler/nir/nir_opcodes.py | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index d69d09d30ce..00720708305 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -90,7 +90,7 @@ class Opcode(object):
 # helper variables for strings
 tfloat = "float"
 tint = "int"
-tbool = "bool32"
+tbool32 = "bool32"
 tuint = "uint"
 tuint16 = "uint16"
 tfloat32 = "float32"
@@ -216,10 +216,10 @@ for src_t in [tint, tuint, tfloat]:
 
 # We'll hand-code the to/from bool conversion opcodes.  Because bool doesn't
 # have multiple bit-sizes, we can always infer the size from the other type.
-unop_convert("f2b", tbool, tfloat, "src0 != 0.0")
-unop_convert("i2b", tbool, tint, "src0 != 0")
-unop_convert("b2f", tfloat, tbool, "src0 ? 1.0 : 0.0")
-unop_convert("b2i", tint, tbool, "src0 ? 1 : 0")
+unop_convert("f2b", tbool32, tfloat, "src0 != 0.0")
+unop_convert("i2b", tbool32, tint, "src0 != 0")
+unop_convert("b2f", tfloat, tbool32, "src0 ? 1.0 : 0.0")
+unop_convert("b2i", tint, tbool32, "src0 ? 1 : 0")
 
 
 # Unary floating-point rounding operations.
@@ -430,7 +430,7 @@ def binop(name, ty, alg_props, const_expr):
binop_convert(name, ty, ty, alg_props, const_expr)
 
 def binop_compare(name, ty, alg_props, const_expr):
-   binop_convert(name, tbool, ty, alg_props, const_expr)
+   binop_convert(name, tbool32, ty, alg_props, const_expr)
 
 def binop_horiz(name, out_size, out_type, src1_size, src1_type, src2_size,
 src2_type, const_expr):
@@ -525,13 +525,13 @@ binop_compare("uge", tuint, "", "src0 >= src1")
 
 # integer-aware GLSL-style comparisons that compare floats and ints
 
-binop_reduce("ball_fequal",  1, tbool, tfloat, "{src0} == {src1}",
+binop_reduce("ball_fequal",  1, tbool32, tfloat, "{src0} == {src1}",
  "{src0} && {src1}", "{src}")
-binop_reduce("bany_fnequal", 1, tbool, tfloat, "{src0} != {src1}",
+binop_reduce("bany_fnequal", 1, tbool32, tfloat, "{src0} != {src1}",
  "{src0} || {src1}", "{src}")
-binop_reduce("ball_iequal",  1, tbool, tint, "{src0} == {src1}",
+binop_reduce("ball_iequal",  1, tbool32, tint, "{src0} == {src1}",
  "{src0} && {src1}", "{src}")
-binop_reduce("bany_inequal", 1, tbool, tint, "{src0} != {src1}",
+binop_reduce("bany_inequal", 1, tbool32, tint, "{src0} != {src1}",
  "{src0} || {src1}", "{src}")
 
 # non-integer-aware GLSL-style comparisons that return 0.0 or 1.0
@@ -719,7 +719,7 @@ triop("imed3", tint, "MAX2(MIN2(MAX2(src0, src1), src2), 
MIN2(src0, src1))")
 triop("umed3", tuint, "MAX2(MIN2(MAX2(src0, src1), src2), MIN2(src0, src1))")
 
 opcode("bcsel", 0, tuint, [0, 0, 0],
-  [tbool, tuint, tuint], "", "src0 ? src1 : src2")
+  [tbool32, tuint, tuint], "", "src0 ? src1 : src2")
 
 # SM5 bfi assembly
 triop("bfi", tuint32, """
-- 
2.19.1

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

[Mesa-dev] [PATCH v2 05/11] nir/algebraic: Add support for unsized conversion opcodes

[Jason Ekstrand]

All conversion opcodes require a destination size but this makes
constructing certain algebraic expressions rather cumbersome.  This
commit adds support to nir_search and nir_algebraic for writing
conversion opcodes without a size.  These meta-opcodes match any
conversion of that type regardless of destination size and the size gets
inferred from the sizes of the things being matched or from other
opcodes in the expression.
---
 src/compiler/nir/nir_algebraic.py | 47 ++--
 src/compiler/nir/nir_search.c | 90 ---
 src/compiler/nir/nir_search.h | 13 -
 3 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 8b137f2a021..def0f94da48 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -33,7 +33,19 @@ import mako.template
 import re
 import traceback
 
-from nir_opcodes import opcodes
+from nir_opcodes import opcodes, type_sizes
+
+# These opcodes are only employed by nir_search.  This provides a mapping from
+# opcode to destination type.
+conv_opcode_types = {
+'i2f' : 'float',
+'u2f' : 'float',
+'f2f' : 'float',
+'f2u' : 'uint',
+'f2i' : 'int',
+'u2u' : 'uint',
+'i2i' : 'int',
+}
 
 if sys.version_info < (3, 0):
 integer_types = (int, long)
@@ -98,7 +110,7 @@ static const ${val.c_type} ${val.name} = {
${val.cond if val.cond else 'NULL'},
 % elif isinstance(val, Expression):
${'true' if val.inexact else 'false'},
-   nir_op_${val.opcode},
+   ${val.c_opcode()},
{ ${', '.join(src.c_ptr for src in val.sources)} },
${val.cond if val.cond else 'NULL'},
 % endif
@@ -276,6 +288,18 @@ class Expression(Value):
   self.sources = [ Value.create(src, "{0}_{1}".format(name_base, i), 
varset)
for (i, src) in enumerate(expr[1:]) ]
 
+  if self.opcode in conv_opcode_types:
+ assert self._bit_size is None, \
+'Expression cannot use an unsized conversion opcode with ' \
+'an explicit size; that\'s silly.'
+
+
+   def c_opcode(self):
+  if self.opcode in conv_opcode_types:
+ return 'nir_search_op_' + self.opcode
+  else:
+ return 'nir_op_' + self.opcode
+
def render(self):
   srcs = "\n".join(src.render() for src in self.sources)
   return srcs + super(Expression, self).render()
@@ -462,6 +486,17 @@ class BitSizeValidator(object):
   if not isinstance(val, Expression):
  return
 
+  # Generic conversion ops are special in that they have a single unsized
+  # source and an unsized destination and the two don't have to match.
+  # This means there's no validation or unioning to do here besides the
+  # len(val.sources) check.
+  if val.opcode in conv_opcode_types:
+ assert len(val.sources) == 1, \
+"Expression {} has {} sources, expected 1".format(
+   val, len(val.sources))
+ self.validate_value(val.sources[0])
+ return
+
   nir_op = opcodes[val.opcode]
   assert len(val.sources) == nir_op.num_inputs, \
  "Expression {} has {} sources, expected {}".format(
@@ -720,7 +755,13 @@ class AlgebraicPass(object):
continue
 
  self.xforms.append(xform)
- self.opcode_xforms[xform.search.opcode].append(xform)
+ if xform.search.opcode in conv_opcode_types:
+dst_type = conv_opcode_types[xform.search.opcode]
+for size in type_sizes(dst_type):
+   sized_opcode = xform.search.opcode + str(size)
+   self.opcode_xforms[sized_opcode].append(xform)
+ else:
+self.opcode_xforms[xform.search.opcode].append(xform)
 
   if error:
  sys.exit(1)
diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c
index a41fca876d5..f5fc92ec33c 100644
--- a/src/compiler/nir/nir_search.c
+++ b/src/compiler/nir/nir_search.c
@@ -89,6 +89,82 @@ src_is_type(nir_src src, nir_alu_type type)
return false;
 }
 
+static bool
+nir_op_matches_search_op(nir_op nop, uint16_t sop)
+{
+   if (sop <= nir_last_opcode)
+  return nop == sop;
+
+#define MATCH_FCONV_CASE(op) \
+   case nir_search_op_##op: \
+  return nop == nir_op_##op##16 || \
+ nop == nir_op_##op##32 || \
+ nop == nir_op_##op##64;
+
+#define MATCH_ICONV_CASE(op) \
+   case nir_search_op_##op: \
+  return nop == nir_op_##op##8 || \
+ nop == nir_op_##op##16 || \
+ nop == nir_op_##op##32 || \
+ nop == nir_op_##op##64;
+
+   switch (sop) {
+   MATCH_FCONV_CASE(i2f)
+   MATCH_FCONV_CASE(u2f)
+   MATCH_FCONV_CASE(f2f)
+   MATCH_ICONV_CASE(f2u)
+   MATCH_ICONV_CASE(f2i)
+   MATCH_ICONV_CASE(u2u)
+   MATCH_ICONV_CASE(i2i)
+   default:
+  unreachable("Invalid nir_search_op");
+   }
+
+#undef MATCH_FCONV_CASE
+#undef MATCH_ICONV_CASE
+}
+
+static nir_op
+nir_op_for_search_op(uint16_t sop, 

[Mesa-dev] [PATCH v2 11/11] FIXUP: Fix NIR producers and consumers to use unsized conversions

[Jason Ekstrand]

---
 src/amd/common/ac_nir_to_llvm.c   | 12 
 src/broadcom/compiler/nir_to_vir.c|  8 
 src/compiler/glsl/glsl_to_nir.cpp |  2 +-
 src/compiler/nir/nir_builder.h| 12 
 src/compiler/spirv/vtn_glsl450.c  |  4 ++--
 src/freedreno/ir3/ir3_compiler_nir.c  | 11 +++
 src/gallium/drivers/vc4/vc4_program.c |  8 
 src/intel/compiler/brw_fs_nir.cpp | 19 ---
 src/intel/compiler/brw_vec4_nir.cpp   |  9 +
 src/mesa/program/prog_to_nir.c|  4 ++--
 10 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index a109f5a8156..fe65dfff8f3 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -941,16 +941,20 @@ static void visit_alu(struct ac_nir_context *ctx, const 
nir_alu_instr *instr)
src[1] = ac_to_integer(>ac, src[1]);
result = emit_uint_carry(>ac, 
"llvm.usub.with.overflow.i32", src[0], src[1]);
break;
-   case nir_op_b2f:
+   case nir_op_b2f16:
+   case nir_op_b2f32:
+   case nir_op_b2f64:
result = emit_b2f(>ac, src[0], 
instr->dest.dest.ssa.bit_size);
break;
-   case nir_op_f2b:
+   case nir_op_f2b32:
result = emit_f2b(>ac, src[0]);
break;
-   case nir_op_b2i:
+   case nir_op_b2i16:
+   case nir_op_b2i32:
+   case nir_op_b2i64:
result = emit_b2i(>ac, src[0], 
instr->dest.dest.ssa.bit_size);
break;
-   case nir_op_i2b:
+   case nir_op_i2b32:
src[0] = ac_to_integer(>ac, src[0]);
result = emit_i2b(>ac, src[0]);
break;
diff --git a/src/broadcom/compiler/nir_to_vir.c 
b/src/broadcom/compiler/nir_to_vir.c
index 57be43d7245..fbe8af376a7 100644
--- a/src/broadcom/compiler/nir_to_vir.c
+++ b/src/broadcom/compiler/nir_to_vir.c
@@ -682,14 +682,14 @@ ntq_emit_alu(struct v3d_compile *c, nir_alu_instr *instr)
 case nir_op_u2f32:
 result = vir_UTOF(c, src[0]);
 break;
-case nir_op_b2f:
+case nir_op_b2f32:
 result = vir_AND(c, src[0], vir_uniform_f(c, 1.0));
 break;
-case nir_op_b2i:
+case nir_op_b2i32:
 result = vir_AND(c, src[0], vir_uniform_ui(c, 1));
 break;
-case nir_op_i2b:
-case nir_op_f2b:
+case nir_op_i2b32:
+case nir_op_f2b32:
 vir_PF(c, src[0], V3D_QPU_PF_PUSHZ);
 result = vir_MOV(c, vir_SEL(c, V3D_QPU_COND_IFNA,
 vir_uniform_ui(c, ~0),
diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index 55628dd2ccd..8237a66eb3f 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -1533,7 +1533,7 @@ nir_visitor::visit(ir_expression *ir)
   result = supports_ints ? nir_u2f32(, srcs[0]) : nir_fmov(, srcs[0]);
   break;
case ir_unop_b2f:
-  result = supports_ints ? nir_b2f(, srcs[0]) : nir_fmov(, srcs[0]);
+  result = supports_ints ? nir_b2f32(, srcs[0]) : nir_fmov(, srcs[0]);
   break;
case ir_unop_f2i:
case ir_unop_f2u:
diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h
index 30fa1d7ec8b..e0cdcd4ba23 100644
--- a/src/compiler/nir/nir_builder.h
+++ b/src/compiler/nir/nir_builder.h
@@ -963,6 +963,18 @@ nir_load_param(nir_builder *build, uint32_t param_idx)
 
 #include "nir_builder_opcodes.h"
 
+static inline nir_ssa_def *
+nir_f2b(nir_builder *build, nir_ssa_def *f)
+{
+   return nir_f2b32(build, f);
+}
+
+static inline nir_ssa_def *
+nir_i2b(nir_builder *build, nir_ssa_def *i)
+{
+   return nir_i2b32(build, i);
+}
+
 static inline nir_ssa_def *
 nir_load_barycentric(nir_builder *build, nir_intrinsic_op op,
  unsigned interp_mode)
diff --git a/src/compiler/spirv/vtn_glsl450.c b/src/compiler/spirv/vtn_glsl450.c
index 06a49e48e3f..b54aeb9b217 100644
--- a/src/compiler/spirv/vtn_glsl450.c
+++ b/src/compiler/spirv/vtn_glsl450.c
@@ -274,7 +274,7 @@ build_atan(nir_builder *b, nir_ssa_def *y_over_x)
/* range-reduction fixup */
tmp = nir_fadd(b, tmp,
   nir_fmul(b,
-   nir_b2f(b, nir_flt(b, one, abs_y_over_x)),
+   nir_b2f32(b, nir_flt(b, one, abs_y_over_x)),
nir_fadd(b, nir_fmul(b, tmp,
 nir_imm_float(b, -2.0f)),
nir_imm_float(b, M_PI_2f;
@@ -346,7 +346,7 @@ build_atan2(nir_builder *b, nir_ssa_def *y, nir_ssa_def *x)
/* Calculate the arctangent and fix up the result if we had flipped the
 * coordinate system.
 */
-   nir_ssa_def *arc = nir_fadd(b, nir_fmul(b, nir_b2f(b, flip),
+   nir_ssa_def *arc = nir_fadd(b, 

[Mesa-dev] [PATCH v2 10/11] FIXUP: nir/opt_algebraic: Add suffixes to some x2b opcodes

[Jason Ekstrand]

Many of the x2b optimizations in nir_opt_algebraic can be handled by the
generic untyped conversion opcodes we just added.  However, there are a
few that still need an explicit size for some reason.
---
 src/compiler/nir/nir_opt_algebraic.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/compiler/nir/nir_opt_algebraic.py 
b/src/compiler/nir/nir_opt_algebraic.py
index aa1a7a94e6e..28e8a96d4d5 100644
--- a/src/compiler/nir/nir_opt_algebraic.py
+++ b/src/compiler/nir/nir_opt_algebraic.py
@@ -440,7 +440,7 @@ optimizations = [
(('fsat', ('fadd', ('b2f', 'a@32'), ('b2f', 'b@32'))), ('b2f', ('ior', a, 
b))),
(('iand', 'a@bool', 1.0), ('b2f', a), '!options->lower_b2f'),
# True/False are ~0 and 0 in NIR.  b2i of True is 1, and -1 is ~0 (True).
-   (('ineg', ('b2i@32', 'a@32')), a),
+   (('ineg', ('b2i32', 'a@32')), a),
(('flt', ('fneg', ('b2f', 'a@32')), 0), a), # Generated by TGSI KILL_IF.
(('flt', ('fsub', 0.0, ('b2f', 'a@32')), 0), a), # Generated by TGSI 
KILL_IF.
# Comparison with the same args.  Note that these are not done for
@@ -532,15 +532,15 @@ optimizations = [
(('fcsel', a, b, b), b),
 
# Conversions
-   (('i2b', ('b2i', 'a@32')), a),
-   (('i2b', 'a@bool'), a),
+   (('i2b32', ('b2i', 'a@32')), a),
+   (('i2b32', 'a@bool'), a),
(('f2i', ('ftrunc', a)), ('f2i', a)),
(('f2u', ('ftrunc', a)), ('f2u', a)),
(('i2b', ('ineg', a)), ('i2b', a)),
(('i2b', ('iabs', a)), ('i2b', a)),
(('fabs', ('b2f', a)), ('b2f', a)),
(('iabs', ('b2i', a)), ('b2i', a)),
-   (('inot', ('f2b', a)), ('feq', a, 0.0)),
+   (('inot', ('f2b32', a)), ('feq', a, 0.0)),
 
# Ironically, mark these as imprecise because removing the conversions may
# preserve more precision than doing the conversions (e.g.,
@@ -754,8 +754,8 @@ for left, right in 
itertools.combinations_with_replacement(invert.keys(), 2):
  ('ior', (invert[left], a, b), (invert[right], c, d
 
 # Optimize x2yN(b2x(x)) -> b2y
-optimizations.append((('f2b', ('b2f', 'a@32')), a))
-optimizations.append((('i2b', ('b2i', 'a@32')), a))
+optimizations.append((('f2b32', ('b2f', 'a@32')), a))
+optimizations.append((('i2b32', ('b2i', 'a@32')), a))
 for x, y in itertools.product(['f', 'u', 'i'], ['f', 'u', 'i']):
if x != 'f' and y != 'f' and x != y:
   continue
@@ -916,7 +916,7 @@ late_optimizations = [
(('fmax', ('fadd(is_used_once)', '#c', a), ('fadd(is_used_once)', '#c', 
b)), ('fadd', c, ('fmax', a, b))),
 
# Lowered for backends without a dedicated b2f instruction
-   (('b2f@32', 'a@32'), ('iand', a, 1.0), 'options->lower_b2f'),
+   (('b2f32', 'a@32'), ('iand', a, 1.0), 'options->lower_b2f'),
 ]
 
 print(nir_algebraic.AlgebraicPass("nir_opt_algebraic", optimizations).render())
-- 
2.19.1

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

[Mesa-dev] [PATCH v2 00/11] nir: Rework boolean conversions

[Jason Ekstrand]

This is mostly a re-send of my earlier series to rework boolean conversions
in NIR which can be found here:

https://lists.freedesktop.org/archives/mesa-dev/2018-November/209089.html

This version solidly chooses path B from the previous series and is rebased
on top of Connor's nir_algebraic.py reworks.  With his rework, the
nir_algebraic.py changes required by this series are much simpler.

Jason Ekstrand (11):
  nir/opcodes: Pull in the type helpers from constant_expressions
  nir/opcodes: Rename tbool to tbool32
  nir/algebraic: Clean up some __str__ cruft
  nir/algebraic: Refactor codegen a bit
  nir/algebraic: Add support for unsized conversion opcodes
  nir/opt_algebraic: Simplify an optimization using the new search ops
  nir/opt_algebraic: Drop bit-size suffixes from conversions
  nir/opt_algebraic: Add 32-bit specifiers to a bunch of booleans
  nir: Make boolean conversions sized just like the others
  FIXUP: nir/opt_algebraic: Add suffixes to some x2b opcodes
  FIXUP: Fix NIR producers and consumers to use unsized conversions

 src/amd/common/ac_nir_to_llvm.c  |  12 +-
 src/broadcom/compiler/nir_to_vir.c   |   8 +-
 src/compiler/glsl/glsl_to_nir.cpp|   2 +-
 src/compiler/nir/nir.h   |   4 +-
 src/compiler/nir/nir_algebraic.py|  74 ++--
 src/compiler/nir/nir_builder.h   |  12 ++
 src/compiler/nir/nir_constant_expressions.py |  25 +--
 src/compiler/nir/nir_lower_idiv.c|   2 +-
 src/compiler/nir/nir_lower_int64.c   |   2 +-
 src/compiler/nir/nir_opcodes.py  |  73 +---
 src/compiler/nir/nir_opcodes_c.py|  41 ++--
 src/compiler/nir/nir_opt_algebraic.py| 187 +--
 src/compiler/nir/nir_opt_if.c|   2 +-
 src/compiler/nir/nir_search.c| 109 ++-
 src/compiler/nir/nir_search.h|  17 +-
 src/compiler/spirv/vtn_glsl450.c |   4 +-
 src/freedreno/ir3/ir3_compiler_nir.c |  11 +-
 src/gallium/drivers/vc4/vc4_program.c|   8 +-
 src/intel/compiler/brw_fs_nir.cpp|  19 +-
 src/intel/compiler/brw_vec4_nir.cpp  |   9 +-
 src/mesa/program/prog_to_nir.c   |   4 +-
 21 files changed, 397 insertions(+), 228 deletions(-)

-- 
2.19.1

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

Re: [Mesa-dev] Lets talk about autotools

[Dylan Baker]

Quoting Timo Aaltonen (2018-11-29 10:33:30)
> On 29.11.2018 20.22, Matt Turner wrote:
> > On Thu, Nov 29, 2018 at 9:47 AM Emil Velikov  
> > wrote:
> >> In Mesa, we have different parts not used by different teams. As such
> >> we tend to remove stuff when nobody is around to maintain it anymore.
> > 
> > We drop things for that reason, but also when something is no longer
> > needed. I don't think autotools is needed. As far as I know all
> > distributions have switched to Meson quite some time ago. I just
> > removed the last version of Mesa from Gentoo that we were still
> > building with autotools.
> 
> FTR, I converted the Debian Mesa packaging to Meson today, and realized
> that the feature in the upcoming Meson is not needed at all by the
> packaging (it can find llvm-config-N.N just fine..). So I'm fine with
> removing autotools support :)

Yay! (meson LLVM integration maintainer here) we support both the Debian style
llvm-config-$version and FreeBSD style llvm-config$version. The only thing
you'll run into is it always tries to select the newest version, you need the
upcoming feature if you want to build against an older version of LLVM with a
newer version installed.

Dylan


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

Re: [Mesa-dev] [PATCH mesa 1/2] docs: add meson cross compilation instructions

[Dylan Baker]

Quoting Eric Engestrom (2018-11-29 07:50:58)
> Signed-off-by: Eric Engestrom 
> ---
> The 32-bit linux one is tested, but the other two are "I think this
> should be right".
> ---
>  docs/meson.html | 83 +
>  1 file changed, 83 insertions(+)
> 
> diff --git a/docs/meson.html b/docs/meson.html
> index 68f80d6ac42d6f48d359..baa1f0f327e99d86e9a5 100644
> --- a/docs/meson.html
> +++ b/docs/meson.html
> @@ -16,6 +16,11 @@ The Mesa 3D Graphics Library
>  
>  Compilation and Installation using Meson
>  
> +
> +  Basic Usage
> +  Cross-compilation and 32-bit 
> builds
> +
> +
>  1. Basic Usage
>  
>  The Meson build system is generally considered stable and ready
> @@ -190,6 +195,84 @@ 1. Basic Usage
>  
>  
>  
> +2. Cross-compilation and 32-bit builds
> +
> +https://mesonbuild.com/Cross-compilation.html;>Meson supports
> +cross-compilation by specifying a number of binary paths and
> +settings in a file and passing this file to meson or
> +meson configure with the --cross-file
> +parameter.
> +
> +This file can live at any location, but you can use the bare filename
> +(without the folder path) if you put it in $XDG_DATA_HOME/meson/cross or
> +~/.local/share/meson/cross
> +
> +Below are a few example of cross files, but keep in mind that you
> +will likely have to alter them for your system.
> +
> +
> +32-bit build on x86 linux:
> +
> +[binaries]
> +c = '/usr/bin/gcc'
> +cpp = '/usr/bin/g++'
> +ar = '/usr/bin/ar'

It's better if possible to use gcc-ar instead of ar, you can't LTO without
gcc-ar, for example.

Without or without the above,
Reviewed-by: Dylan Baker 

> +strip = '/usr/bin/strip'
> +pkgconfig = '/usr/bin/pkg-config-32'
> +llvm-config = '/usr/bin/llvm-config'
> +
> +[properties]
> +c_args = ['-m32']
> +c_link_args = ['-m32']
> +cpp_args = ['-m32']
> +cpp_link_args = ['-m32']
> +
> +[host_machine]
> +system = 'linux'
> +cpu_family = 'x86'
> +cpu = 'i686'
> +endian = 'little'
> +
> +
> +
> +
> +64-bit build on ARM linux:
> +
> +[binaries]
> +c = '/usr/bin/aarch64-linux-gnu-gcc'
> +cpp = '/usr/bin/aarch64-linux-gnu-g++'
> +ar = '/usr/bin/aarch64-linux-gnu-ar'
> +strip = '/usr/bin/aarch64-linux-gnu-strip'
> +pkgconfig = '/usr/bin/aarch64-linux-gnu-pkg-config'
> +exe_wrapper = '/usr/bin/qemu-aarch64-static'
> +
> +[host_machine]
> +system = 'linux'
> +cpu_family = 'arm'
> +cpu = 'aarch64'
> +endian = 'little'
> +
> +
> +
> +
> +64-bit build on x86 windows:
> +
> +[binaries]
> +c = '/usr/bin/x86_64-w64-mingw32-gcc'
> +cpp = '/usr/bin/x86_64-w64-mingw32-g++'
> +ar = '/usr/bin/x86_64-w64-mingw32-ar'
> +strip = '/usr/bin/x86_64-w64-mingw32-strip'
> +pkgconfig = '/usr/bin/x86_64-w64-mingw32-pkg-config'
> +exe_wrapper = 'wine'
> +
> +[host_machine]
> +system = 'windows'
> +cpu_family = 'x86_64'
> +cpu = 'i686'
> +endian = 'little'
> +
> +
> +
>  
>  
>  
> -- 
> Cheers,
>   Eric
> 


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

Re: [Mesa-dev] [PATCH mesa] meson: skip asm check when asm is disabled

[Dylan Baker]

Quoting Eric Engestrom (2018-11-29 03:50:48)
> Signed-off-by: Eric Engestrom 
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index 4ee40503d044a0985c89..2d300d0421980cba7866 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -917,7 +917,7 @@ endif
>  # case of cross compiling where we can use asm, and that's x86_64 -> x86 when
>  # host OS == build OS, since in that case the build machine can run the 
> host's
>  # binaries.
> -if meson.is_cross_build() 
> +if with_asm and meson.is_cross_build()
>if build_machine.system() != host_machine.system()
>  # TODO: It may be possible to do this with an exe_wrapper (like wine).
>  message('Cross compiling from one OS to another, disabling assembly.')
> -- 
> Cheers,
>   Eric
> 

Reviewed-by: Dylan Baker 


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

Re: [Mesa-dev] [PATCH v2] anv: flush pipeline before query result copies

[Lionel Landwerlin]

On 29/11/2018 16:58, Jason Ekstrand wrote:
On Thu, Nov 29, 2018 at 10:45 AM Lionel Landwerlin 
mailto:lionel.g.landwer...@intel.com>> 
wrote:


Pipeline barriers inserted through vkCmdPipelineBarrier() should be
taken into account when copying results.

In the particular bug below, the results of the
vkCmdCopyQueryPoolResults() command was being overwritten by the
preceding vkCmdCopyBuffer() with a same destination buffer. This is
because we copy the buffers using the 3D pipeline whereas we copy the
query results using the command streamer. Those work in parallel
unless synchronized.

v2: Unconditionally flush the pipeline before copying the results
    (Jason)

Signed-off-by: Lionel Landwerlin mailto:lionel.g.landwer...@intel.com>>
Suggested-by: Jason Ekstrand mailto:ja...@jlekstrand.net>>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
Cc: mesa-sta...@lists.freedesktop.org

---
 src/intel/vulkan/genX_query.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/intel/vulkan/genX_query.c
b/src/intel/vulkan/genX_query.c
index ce8757f2643..f1472c0d570 100644
--- a/src/intel/vulkan/genX_query.c
+++ b/src/intel/vulkan/genX_query.c
@@ -729,11 +729,10 @@ void genX(CmdCopyQueryPoolResults)(
    ANV_FROM_HANDLE(anv_query_pool, pool, queryPool);
    ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);

-   if (flags & VK_QUERY_RESULT_WAIT_BIT) {
-      anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
-         pc.CommandStreamerStallEnable = true;
-         pc.StallAtPixelScoreboard     = true;
-      }
+   if (flags & VK_QUERY_RESULT_WAIT_BIT ||


That bare & should have some parens.  With that,

Reviewed-by: Jason Ekstrand >



Thanks, pushed with your requested change.


-

Lionel



+       (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
+      cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+      genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
    }

    struct anv_address dest_addr =
anv_address_add(buffer->address, destOffset);
-- 
2.20.0.rc1




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

Re: [Mesa-dev] [PATCH 1/2] nir/algebraic: Rewrite bit-size inference

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 12:32 PM Connor Abbott  wrote:

> Before this commit, there were two copies of the algorithm: one in C,
> that we would use to figure out what bit-size to give the replacement
> expression, and one in Python, that emulated the C one and tried to
> prove that the C algorithm would never fail to correctly assign
> bit-sizes. That seemed pretty fragile, and likely to fall over if we
> make any changes. Furthermore, the C code was really just recomputing
> more-or-less the same thing as the Python code every time. Instead, we
> can just store the results of the Python algorithm in the C
> datastructure, and consult it to compute the bitsize of each value,
> moving the "brains" entirely into Python. Since the Python algorithm no
> longer has to match C, it's also a lot easier to change it to something
> more closely approximating an actual type-inference algorithm. The
> algorithm used is based on Hindley-Milner, although deliberately
> weakened a little. It's a few more lines than the old one, judging by
> the diffstat, but I think it's easier to verify that it's correct while
> being as general as possible.
>
> We could split this up into two changes, first making the C code use the
> results of the Python code and then rewriting the Python algorithm, but
> since the old algorithm never tracked which variable each equivalence
> class, it would mean we'd have to add some non-trivial code which would
> then get thrown away. I think it's better to see the final state all at
> once, although I could also try splitting it up.
> ---
>  src/compiler/nir/nir_algebraic.py | 518 --
>  src/compiler/nir/nir_search.c | 146 +
>  src/compiler/nir/nir_search.h |   2 +-
>  3 files changed, 295 insertions(+), 371 deletions(-)
>
> diff --git a/src/compiler/nir/nir_algebraic.py
> b/src/compiler/nir/nir_algebraic.py
> index 728196136ab..48390dbde38 100644
> --- a/src/compiler/nir/nir_algebraic.py
> +++ b/src/compiler/nir/nir_algebraic.py
> @@ -88,7 +88,7 @@ class Value(object):
>
> __template = mako.template.Template("""
>  static const ${val.c_type} ${val.name} = {
> -   { ${val.type_enum}, ${val.bit_size} },
> +   { ${val.type_enum}, ${val.c_bit_size} },
>  % if isinstance(val, Constant):
> ${val.type()}, { ${val.hex()} /* ${val.value} */ },
>  % elif isinstance(val, Variable):
> @@ -112,6 +112,40 @@ static const ${val.c_type} ${val.name} = {
> def __str__(self):
>return self.in_val
>
> +   def get_bit_size(self):
> +  """Get the physical bit-size that has been chosen for this value,
> or if
> +  there is none, the canonical value which currently represents this
> +  bit-size class. Variables will be preferred, i.e. if there are any
> +  variables in the equivalence class, the canonical value will be a
> +  variable. We do this since we'll need to know which variable each
> value
> +  is equivalent to when constructing the replacement expression. This
> is
> +  the "find" part of the union-find algorithm.
> +  """
> +  bit_size = self
> +
> +  while isinstance(bit_size, Value):
> + if bit_size._bit_size == None:
> +break
> + bit_size = bit_size._bit_size
> +
> +  if bit_size != self:
> + self._bit_size = bit_size
> +  return bit_size
> +
> +   def set_bit_size(self, other):
> +  """Make self.get_bit_size() return what other.get_bit_size() return
> +  before calling this, or just "other" if it's a concrete bit-size.
> This is
> +  the "union" part of the union-find algorithm.
> +  """
> +
> +  self_bit_size = self.get_bit_size()
> +  other_bit_size = other if isinstance(other, int) else
> other.get_bit_size()
> +
> +  if self_bit_size == other_bit_size:
> + return
> +
> +  self_bit_size._bit_size = other_bit_size
> +
> @property
> def type_enum(self):
>return "nir_search_value_" + self.type_str
> @@ -124,6 +158,21 @@ static const ${val.c_type} ${val.name} = {
> def c_ptr(self):
>return "&{0}.value".format(self.name)
>
> +   @property
> +   def c_bit_size(self):
> +  bit_size = self.get_bit_size()
> +  if isinstance(bit_size, int):
> + return bit_size
> +  elif isinstance(bit_size, Variable):
> + return -bit_size.index - 1
> +  else:
> + # If the bit-size class is neither a variable, nor an actual
> bit-size, then
> + # - If it's in the search expression, we don't need to check
> anything
> + # - If it's in the replace expression, either it's ambiguous (in
> which
> + # case we'd reject it), or it equals the bit-size of the search
> value
> + # We represent these cases with a 0 bit-size.
> + return 0
> +
> def render(self):
>return self.__template.render(val=self,
>  Constant=Constant,
> @@ -140,14 +189,14 @@ class Constant(Value):
>if isinstance(val, (str)):

Re: [Mesa-dev] [PATCH] st/mesa: expose EXT_render_snorm on GLES

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 
On Thu, Nov 29, 2018 at 3:36 PM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> ---
>  docs/relnotes/19.0.0.html  | 1 +
>  src/mesa/state_tracker/st_extensions.c | 8 
>  2 files changed, 9 insertions(+)
>
> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index f4522b654c6..04e06cdd758 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -36,20 +36,21 @@ TBD.
>  
>
>
>  New features
>
>  
>  GL_AMD_texture_texture4 on all GL 4.0 drivers.
>  GL_EXT_shader_implicit_conversions on all drivers (ES extension).
>  GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES 
> extension).
>  GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES 
> extension).
> +GL_EXT_render_snorm on gallium drivers (ES extension).
>  GL_EXT_texture_view on drivers supporting texture views (ES 
> extension).
>  GL_OES_texture_view on drivers supporting texture views (ES 
> extension).
>  
>
>  Bug fixes
>
>  
>  TBD
>  
>
> diff --git a/src/mesa/state_tracker/st_extensions.c 
> b/src/mesa/state_tracker/st_extensions.c
> index 5c068d14e93..33ce61d779b 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -796,20 +796,28 @@ void st_init_extensions(struct pipe_screen *screen,
>{ { o(EXT_packed_float) },
>  { PIPE_FORMAT_R11G11B10_FLOAT } },
>
>{ { o(EXT_texture_integer) },
>  { PIPE_FORMAT_R32G32B32A32_UINT,
>PIPE_FORMAT_R32G32B32A32_SINT } },
>
>{ { o(ARB_texture_rg) },
>  { PIPE_FORMAT_R8_UNORM,
>PIPE_FORMAT_R8G8_UNORM } },
> +
> +  { { o(EXT_render_snorm) },
> +{ PIPE_FORMAT_R8_SNORM,
> +  PIPE_FORMAT_R8G8_SNORM,
> +  PIPE_FORMAT_R8G8B8A8_SNORM,
> +  PIPE_FORMAT_R16_SNORM,
> +  PIPE_FORMAT_R16G16_SNORM,
> +  PIPE_FORMAT_R16G16B16A16_SNORM } },
> };
>
> /* Required: depth stencil and sampler support */
> static const struct st_extension_format_mapping depthstencil_mapping[] = {
>{ { o(ARB_depth_buffer_float) },
>  { PIPE_FORMAT_Z32_FLOAT,
>PIPE_FORMAT_Z32_FLOAT_S8X24_UINT } },
> };
>
> /* Required: sampler support */
> --
> 2.17.1
>
> ___
> 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] [PATCH] st/mesa: expose EXT_render_snorm on GLES

[Marek Olšák]

From: Marek Olšák 

---
 docs/relnotes/19.0.0.html  | 1 +
 src/mesa/state_tracker/st_extensions.c | 8 
 2 files changed, 9 insertions(+)

diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index f4522b654c6..04e06cdd758 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -36,20 +36,21 @@ TBD.
 
 
 
 New features
 
 
 GL_AMD_texture_texture4 on all GL 4.0 drivers.
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
 GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension).
 GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
+GL_EXT_render_snorm on gallium drivers (ES extension).
 GL_EXT_texture_view on drivers supporting texture views (ES 
extension).
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
 
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 5c068d14e93..33ce61d779b 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -796,20 +796,28 @@ void st_init_extensions(struct pipe_screen *screen,
   { { o(EXT_packed_float) },
 { PIPE_FORMAT_R11G11B10_FLOAT } },
 
   { { o(EXT_texture_integer) },
 { PIPE_FORMAT_R32G32B32A32_UINT,
   PIPE_FORMAT_R32G32B32A32_SINT } },
 
   { { o(ARB_texture_rg) },
 { PIPE_FORMAT_R8_UNORM,
   PIPE_FORMAT_R8G8_UNORM } },
+
+  { { o(EXT_render_snorm) },
+{ PIPE_FORMAT_R8_SNORM,
+  PIPE_FORMAT_R8G8_SNORM,
+  PIPE_FORMAT_R8G8B8A8_SNORM,
+  PIPE_FORMAT_R16_SNORM,
+  PIPE_FORMAT_R16G16_SNORM,
+  PIPE_FORMAT_R16G16B16A16_SNORM } },
};
 
/* Required: depth stencil and sampler support */
static const struct st_extension_format_mapping depthstencil_mapping[] = {
   { { o(ARB_depth_buffer_float) },
 { PIPE_FORMAT_Z32_FLOAT,
   PIPE_FORMAT_Z32_FLOAT_S8X24_UINT } },
};
 
/* Required: sampler support */
-- 
2.17.1

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

Re: [Mesa-dev] Lets talk about autotools

[Timo Aaltonen]

On 29.11.2018 20.22, Matt Turner wrote:
> On Thu, Nov 29, 2018 at 9:47 AM Emil Velikov  wrote:
>> In Mesa, we have different parts not used by different teams. As such
>> we tend to remove stuff when nobody is around to maintain it anymore.
> 
> We drop things for that reason, but also when something is no longer
> needed. I don't think autotools is needed. As far as I know all
> distributions have switched to Meson quite some time ago. I just
> removed the last version of Mesa from Gentoo that we were still
> building with autotools.

FTR, I converted the Debian Mesa packaging to Meson today, and realized
that the feature in the upcoming Meson is not needed at all by the
packaging (it can find llvm-config-N.N just fine..). So I'm fine with
removing autotools support :)


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

[Mesa-dev] [PATCH 2/2] nir/algebraic: Add unit tests for bitsize validation

[Connor Abbott]

The non-failure path can be tested by just compiling mesa and then
testing it, but the failure paths won't be hit unless you make a mistake,
so it's best to test them with some unit tests.
---
 src/compiler/Makefile.nir.am  |   4 +-
 src/compiler/nir/meson.build  |   7 ++
 .../nir/tests/algebraic_parser_test.py| 116 ++
 .../nir/tests/algebraic_parser_test.sh|   3 +
 4 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 src/compiler/nir/tests/algebraic_parser_test.py
 create mode 100644 src/compiler/nir/tests/algebraic_parser_test.sh

diff --git a/src/compiler/Makefile.nir.am b/src/compiler/Makefile.nir.am
index c646c6bdc1e..aa0a92856f1 100644
--- a/src/compiler/Makefile.nir.am
+++ b/src/compiler/Makefile.nir.am
@@ -87,10 +87,12 @@ nir_tests_vars_tests_SOURCES = nir/tests/vars_tests.cpp
 nir_tests_vars_tests_CFLAGS = $(NIR_TESTS_CFLAGS)
 nir_tests_vars_tests_LDADD = $(NIR_TESTS_LDADD)
 
+check_SCRIPTS = nir/tests/algebraic_parser_test.sh
 
 TESTS += \
 nir/tests/control_flow_tests \
-nir/tests/vars_tests
+nir/tests/vars_tests \
+   nir/tests/algebraic_parser_test.sh
 
 
 BUILT_SOURCES += \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index b0c3a7feb31..84e58cafb6f 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -259,4 +259,11 @@ if with_tests
   link_with : libmesa_util,
 )
   )
+  test(
+'nir_algebraic_parser',
+prog_python,
+args : [
+  join_paths(meson.current_source_dir(), 'tests/algebraic_parser_test.py')
+],
+  )
 endif
diff --git a/src/compiler/nir/tests/algebraic_parser_test.py 
b/src/compiler/nir/tests/algebraic_parser_test.py
new file mode 100644
index 000..492a09ec7db
--- /dev/null
+++ b/src/compiler/nir/tests/algebraic_parser_test.py
@@ -0,0 +1,116 @@
+#
+# Copyright (C) 2018 Valve Corporation
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice (including the next
+# paragraph) shall be included in all copies or substantial portions of the
+# Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+# IN THE SOFTWARE.
+
+import unittest
+
+import sys
+import os
+sys.path.insert(1, os.path.join(sys.path[0], '..'))
+
+from nir_algebraic import SearchAndReplace
+
+# These tests check that the bitsize validator correctly rejects various
+# different kinds of malformed expressions, and documents what the error
+# message looks like.
+
+a = 'a'
+b = 'b'
+c = 'c'
+
+class ValidatorTests(unittest.TestCase):
+pattern = ()
+message = ''
+
+def common(self, pattern, message):
+with self.assertRaises(AssertionError) as context:
+SearchAndReplace(pattern)
+
+self.assertEqual(message, str(context.exception))
+
+def test_wrong_src_count(self):
+self.common((('iadd', a), ('fadd', a, a)),
+"Expression ('iadd', 'a') has 1 sources, expected 2")
+
+def test_var_bitsize(self):
+self.common((('iadd', 'a@32', 'a@64'), ('fadd', a, a)),
+"Variable a has conflicting bit size requirements: " \
+"it must have bit size 32 and 64")
+
+def test_var_bitsize_2(self):
+self.common((('iadd', a, 'a@32'), ('fadd', 'a@64', a)),
+"Variable a has conflicting bit size requirements: " \
+"it must have bit size 32 and 64")
+
+def test_search_src_bitsize(self):
+self.common((('iadd', 'a@32', 'b@64'), ('fadd', a, b)),
+"Source a@32 of ('iadd', 'a@32', 'b@64') must have bit size 32, " \
+"while source b@64 must have incompatible bit size 64")
+
+def test_replace_src_bitsize(self):
+self.common((('iadd', a, ('b2i', b)), ('iadd', a, b)),
+"Sources a (bit size of a) and b (bit size of 32) " \
+"of ('iadd', 'a', 'b') may not have the same bit size " \
+"when building the replacement expression.")
+
+def test_search_src_bitsize_fixed(self):
+self.common((('ishl', a, 'b@64'), ('ishl', a, b)),
+"b@64 must have 64 bits, but as a source of 

[Mesa-dev] [PATCH 1/2] nir/algebraic: Rewrite bit-size inference

[Connor Abbott]

Before this commit, there were two copies of the algorithm: one in C,
that we would use to figure out what bit-size to give the replacement
expression, and one in Python, that emulated the C one and tried to
prove that the C algorithm would never fail to correctly assign
bit-sizes. That seemed pretty fragile, and likely to fall over if we
make any changes. Furthermore, the C code was really just recomputing
more-or-less the same thing as the Python code every time. Instead, we
can just store the results of the Python algorithm in the C
datastructure, and consult it to compute the bitsize of each value,
moving the "brains" entirely into Python. Since the Python algorithm no
longer has to match C, it's also a lot easier to change it to something
more closely approximating an actual type-inference algorithm. The
algorithm used is based on Hindley-Milner, although deliberately
weakened a little. It's a few more lines than the old one, judging by
the diffstat, but I think it's easier to verify that it's correct while
being as general as possible.

We could split this up into two changes, first making the C code use the
results of the Python code and then rewriting the Python algorithm, but
since the old algorithm never tracked which variable each equivalence
class, it would mean we'd have to add some non-trivial code which would
then get thrown away. I think it's better to see the final state all at
once, although I could also try splitting it up.
---
 src/compiler/nir/nir_algebraic.py | 518 --
 src/compiler/nir/nir_search.c | 146 +
 src/compiler/nir/nir_search.h |   2 +-
 3 files changed, 295 insertions(+), 371 deletions(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 728196136ab..48390dbde38 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -88,7 +88,7 @@ class Value(object):
 
__template = mako.template.Template("""
 static const ${val.c_type} ${val.name} = {
-   { ${val.type_enum}, ${val.bit_size} },
+   { ${val.type_enum}, ${val.c_bit_size} },
 % if isinstance(val, Constant):
${val.type()}, { ${val.hex()} /* ${val.value} */ },
 % elif isinstance(val, Variable):
@@ -112,6 +112,40 @@ static const ${val.c_type} ${val.name} = {
def __str__(self):
   return self.in_val
 
+   def get_bit_size(self):
+  """Get the physical bit-size that has been chosen for this value, or if
+  there is none, the canonical value which currently represents this
+  bit-size class. Variables will be preferred, i.e. if there are any
+  variables in the equivalence class, the canonical value will be a
+  variable. We do this since we'll need to know which variable each value
+  is equivalent to when constructing the replacement expression. This is
+  the "find" part of the union-find algorithm.
+  """
+  bit_size = self
+
+  while isinstance(bit_size, Value):
+ if bit_size._bit_size == None:
+break
+ bit_size = bit_size._bit_size
+
+  if bit_size != self:
+ self._bit_size = bit_size
+  return bit_size
+
+   def set_bit_size(self, other):
+  """Make self.get_bit_size() return what other.get_bit_size() return
+  before calling this, or just "other" if it's a concrete bit-size. This is
+  the "union" part of the union-find algorithm.
+  """
+
+  self_bit_size = self.get_bit_size()
+  other_bit_size = other if isinstance(other, int) else 
other.get_bit_size()
+
+  if self_bit_size == other_bit_size:
+ return
+
+  self_bit_size._bit_size = other_bit_size
+
@property
def type_enum(self):
   return "nir_search_value_" + self.type_str
@@ -124,6 +158,21 @@ static const ${val.c_type} ${val.name} = {
def c_ptr(self):
   return "&{0}.value".format(self.name)
 
+   @property
+   def c_bit_size(self):
+  bit_size = self.get_bit_size()
+  if isinstance(bit_size, int):
+ return bit_size
+  elif isinstance(bit_size, Variable):
+ return -bit_size.index - 1
+  else:
+ # If the bit-size class is neither a variable, nor an actual 
bit-size, then
+ # - If it's in the search expression, we don't need to check anything
+ # - If it's in the replace expression, either it's ambiguous (in which
+ # case we'd reject it), or it equals the bit-size of the search value
+ # We represent these cases with a 0 bit-size.
+ return 0
+
def render(self):
   return self.__template.render(val=self,
 Constant=Constant,
@@ -140,14 +189,14 @@ class Constant(Value):
   if isinstance(val, (str)):
  m = _constant_re.match(val)
  self.value = ast.literal_eval(m.group('value'))
- self.bit_size = int(m.group('bits')) if m.group('bits') else 0
+ self._bit_size = int(m.group('bits')) if m.group('bits') else None
   else:
  self.value 

[Mesa-dev] [PATCH 0/2] nir/algebraic: Rewrite bit-size handling

[Connor Abbott]

While nir_algebraic in general is great, the code to implement bit-size
inference has always been pretty fragile. While I was trying to remember
how everything worked in order to review Jason's patches touching this
area, I realized that I could make the whole thing significantly
simpler. This series is the end result of that. Since I was nice, I also
included some tests to exercise all the different errors you can hit. It
conflicts a little bit with Jason's series, since the bit size inference
algorithm has to be aware of the new sizeless comparison operators, but
I won't think it'll be too bad. And it means that I don't have to review
the trickiest prep patches for that series :)

This series is available at:
https://gitlab.freedesktop.org/cwabbott0/mesa/commits/nir-bitsize-validator-rewrite
It might be better to go there to see the final version of the
BitSizeValidator, since that part of the diff won't be too helpful.

Finally, I haven't actually tested the compiled mesa, but I'm working on
it :) It would be good to give this a run through the Intel CI.

Connor Abbott (2):
  nir/algebraic: Rewrite bit-size inference
  nir/algebraic: Add unit tests for bitsize validation

 src/compiler/Makefile.nir.am  |   4 +-
 src/compiler/nir/meson.build  |   7 +
 src/compiler/nir/nir_algebraic.py | 518 ++
 src/compiler/nir/nir_search.c | 146 +
 src/compiler/nir/nir_search.h |   2 +-
 .../nir/tests/algebraic_parser_test.py| 116 
 .../nir/tests/algebraic_parser_test.sh|   3 +
 7 files changed, 424 insertions(+), 372 deletions(-)
 create mode 100644 src/compiler/nir/tests/algebraic_parser_test.py
 create mode 100644 src/compiler/nir/tests/algebraic_parser_test.sh

-- 
2.17.2

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

Re: [Mesa-dev] Lets talk about autotools

[Matt Turner]

On Thu, Nov 29, 2018 at 9:47 AM Emil Velikov  wrote:
> In Mesa, we have different parts not used by different teams. As such
> we tend to remove stuff when nobody is around to maintain it anymore.

We drop things for that reason, but also when something is no longer
needed. I don't think autotools is needed. As far as I know all
distributions have switched to Meson quite some time ago. I just
removed the last version of Mesa from Gentoo that we were still
building with autotools.

> That said, I'm planning to continue maintaining it and would appreciate
> if we keep it in-tree.

If it were a driver for some old hardware that you wanted to maintain
I would be sympathetic. But this is an enormous build system that is
going to increase overhead for everyone, and for unclear benefit.

FWIW, git stats on git rm `find -name Makefile.am` configure.ac are

 105 files changed, 10872 deletions(-)

and I bet that's not all that could be removed.

That's the same order of maginitude as nouveau vieux or classic
radeon, and a lot more overhead than both.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 11:37 AM Matt Turner  wrote:

> On Wed, Nov 28, 2018 at 11:30 AM Jason Ekstrand 
> wrote:
> > We have enough stubborn people on the list that MRs are going to
> constantly get pulled back to the list just because someone doesn't want to
> use the web interface.
>
> A couple of people in this thread have now made similar claims, but
> thus far no one has spoken out against MR-based review.
>

If no one minds MR-based review then let's just do it?

I think the point is that the only reason to have the weird hybrid is to
satisfy the people that are against MR-based review.  If those people
exist, then the above problem is real and the hybrid is going to cause
problems.  If they do not exist, then the weird hybrid is pointless.
Therefore (please pardon the formal logic), the MR+list hybrid is a bad
idea regardless of whether or not MR haters exist.

Of course not every contributor has offered an opinion, but I'm not
> sure the claim is still valid.
>

Fair enough.  I was honestly expecting to see way more push-back.  I'm a
bit surprised that the most extreme position expressed so far is Jordan's,
"maybe not so fast".

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

Re: [Mesa-dev] Lets talk about autotools

[Emil Velikov]

Hi all,

I can see why people may opt to not use or maintain the autotools build.
Although I would kindly ask that we do not remove it just yet.

In Mesa, we have different parts not used by different teams. As such
we tend to remove stuff when nobody is around to maintain it anymore.

That said, I'm planning to continue maintaining it and would appreciate
if we keep it in-tree.

As people may be concerned about bugreports and alike we can trivially
add a warning (as configure is invoked) to forwards any issues to my
email. Additionally (or alternatively) we can have an autotools bugzilla
category with me as the default assignee.

What do you guys think?

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

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Matt Turner]

On Wed, Nov 28, 2018 at 11:30 AM Jason Ekstrand  wrote:
> We have enough stubborn people on the list that MRs are going to constantly 
> get pulled back to the list just because someone doesn't want to use the web 
> interface.

A couple of people in this thread have now made similar claims, but
thus far no one has spoken out against MR-based review.

Of course not every contributor has offered an opinion, but I'm not
sure the claim is still valid.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 1/2] docs: add meson cross compilation instructions

[Emil Velikov]

On Thu, 29 Nov 2018 at 15:51, Eric Engestrom  wrote:
>
> Signed-off-by: Eric Engestrom 
> ---
> The 32-bit linux one is tested, but the other two are "I think this
> should be right".

There's a small nit about the 32 bit linux build. But regardless, the series is
Reviewed-by: Emil Velikov 

If any of the examples off, we can correct when we spot it. These are
a really good starting point.


> +ar = '/usr/bin/ar'
AFAICT the archive produced by ar shouldn't vary across CPU
architectures. Although varying filename limitations (imposed by the
file system) are common.

> +llvm-config = '/usr/bin/llvm-config'
llvm-config will vary greatly across distros, but on Arch it should be
llvm-config32,
Things may work with as-is, but I'm not sure how reliable/good idea it is.

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

Re: [Mesa-dev] [PATCH v2] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 10:45 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> v2: Unconditionally flush the pipeline before copying the results
> (Jason)
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Jason Ekstrand 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/intel/vulkan/genX_query.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
> index ce8757f2643..f1472c0d570 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -729,11 +729,10 @@ void genX(CmdCopyQueryPoolResults)(
> ANV_FROM_HANDLE(anv_query_pool, pool, queryPool);
> ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
>
> -   if (flags & VK_QUERY_RESULT_WAIT_BIT) {
> -  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
> - pc.CommandStreamerStallEnable = true;
> - pc.StallAtPixelScoreboard = true;
> -  }
> +   if (flags & VK_QUERY_RESULT_WAIT_BIT ||
>

That bare & should have some parens.  With that,

Reviewed-by: Jason Ekstrand 


> +   (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
> +  cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> +  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
> }
>
> struct anv_address dest_addr = anv_address_add(buffer->address,
> destOffset);
> --
> 2.20.0.rc1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] anv: flush pipeline before query result copies

[Lionel Landwerlin]

Pipeline barriers inserted through vkCmdPipelineBarrier() should be
taken into account when copying results.

In the particular bug below, the results of the
vkCmdCopyQueryPoolResults() command was being overwritten by the
preceding vkCmdCopyBuffer() with a same destination buffer. This is
because we copy the buffers using the 3D pipeline whereas we copy the
query results using the command streamer. Those work in parallel
unless synchronized.

v2: Unconditionally flush the pipeline before copying the results
(Jason)

Signed-off-by: Lionel Landwerlin 
Suggested-by: Jason Ekstrand 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
Cc: mesa-sta...@lists.freedesktop.org
---
 src/intel/vulkan/genX_query.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
index ce8757f2643..f1472c0d570 100644
--- a/src/intel/vulkan/genX_query.c
+++ b/src/intel/vulkan/genX_query.c
@@ -729,11 +729,10 @@ void genX(CmdCopyQueryPoolResults)(
ANV_FROM_HANDLE(anv_query_pool, pool, queryPool);
ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
 
-   if (flags & VK_QUERY_RESULT_WAIT_BIT) {
-  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
- pc.CommandStreamerStallEnable = true;
- pc.StallAtPixelScoreboard = true;
-  }
+   if (flags & VK_QUERY_RESULT_WAIT_BIT ||
+   (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
+  cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}
 
struct anv_address dest_addr = anv_address_add(buffer->address, destOffset);
-- 
2.20.0.rc1

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

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 9:43 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Jason Ekstrand 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/intel/vulkan/genX_query.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
> index ce8757f2643..edf9962179a 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -730,10 +730,9 @@ void genX(CmdCopyQueryPoolResults)(
> ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
>
> if (flags & VK_QUERY_RESULT_WAIT_BIT) {
> -  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
> - pc.CommandStreamerStallEnable = true;
> - pc.StallAtPixelScoreboard = true;
> -  }
> +  if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)
> + cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
> +  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
>

Unfortunately, I think we need to do it somewhat unconditionally.  More
specifically

if ((flags & VK_QUERY_RESULT_WAIT_BIT) ||
(cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)) {
   cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
   genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}

Otherwise, we still have a write-after-write hazard if the client does a
vkCmdFillBuffer to zero out the destination prior to copying query pool
results into it.

--Jason


> }
>
> struct anv_address dest_addr = anv_address_add(buffer->address,
> destOffset);
> --
> 2.20.0.rc1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Józef Kucia]

On Thu, Nov 29, 2018 at 5:25 PM Lionel Landwerlin
 wrote:
> Pretty easy.
>
> Fetch http://source.winehq.org/git/vkd3d.git
>
> Usual autotool compilation.
>
> make tests/d3d12
>
> I commented the tests I wasn't interested in for to make it easier and using 
> vktrace to understand what's going on.

Yeah, it's easy. You just need a relatively recent version of widl in
the path, SPIR-V and Vulkan headers. Widl is usually installed
together with Wine. You can also build widl from wine.git (make
tools/widl), and pass WIDL=/path/to/widl to configure.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Lionel Landwerlin]

On 29/11/2018 16:15, Jason Ekstrand wrote:
On Thu, Nov 29, 2018 at 9:52 AM Józef Kucia > wrote:


On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
mailto:lionel.g.landwer...@intel.com>> wrote:
>
> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we
copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin mailto:lionel.g.landwer...@intel.com>>
> Suggested-by: Jason Ekstrand mailto:ja...@jlekstrand.net>>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894

Thanks for the fix.

For what it's worth, we have more test failures on Anv. A lot failures
are related to clearing multisample array textures. See failures in
test_clear_render_target_view(), if you are interested.


Mind filing a few bugs?  Feel free to bucket them such as "multisample 
array texture clears".  How easy is it to run your tests?


Pretty easy.

Fetch http://source.winehq.org/git/vkd3d.git

Usual autotool compilation.

make tests/d3d12

I commented the tests I wasn't interested in for to make it easier and 
using vktrace to understand what's going on.



-

Lionel

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

Re: [Mesa-dev] [PATCH 1/3] radeonsi: allow si_cp_dma_clear_buffer to clear GDS from any IB

[Koenig, Christian]

Hi Marek,

you stumbled over a pretty fundamental bug in the memory management here. 
Essentially we where leaking BOs when we ran into an OOM situation. Patch to 
fix this is on the mailing list.

A second problem is that eviction doesn't seem to work when GDS BOs aren't 
idle. In other words when two applications try to use GDS at the same time they 
don't wait for each other, but rather one looses with an OOM message.

Currently investigating why this is happening,
Christian.

Am 29.11.18 um 02:29 schrieb Marek Olšák:
Hi Christian,

I just pushed the commits.

The best way to reproduce the out-of-memory errors is to run 2 instances of the 
test simultaneously:

R600_DEBUG=testgdsmm glxgears &
R600_DEBUG=testgdsmm glxgears &

It takes about 10 seconds to finish and you'll get a lot of errors.

If you run it again, all GDS allocations will fail:

R600_DEBUG=testgdsmm glxgears
amdgpu: Failed to allocate a buffer:
amdgpu:size  : 32768 bytes
amdgpu:alignment : 4 bytes
amdgpu:domains   : 8

Marek

On Wed, Nov 28, 2018 at 2:16 PM Christian König 
mailto:ckoenig.leichtzumer...@gmail.com>> 
wrote:
Are those committed yet? They don't seem to apply cleanly on master.

Christian.

Am 27.11.18 um 02:56 schrieb Marek Olšák:
> From: Marek Olšák mailto:marek.ol...@amd.com>>
>
> ---
>   .../drivers/radeonsi/si_compute_blit.c|  4 +-
>   src/gallium/drivers/radeonsi/si_cp_dma.c  | 49 ++-
>   src/gallium/drivers/radeonsi/si_pipe.h|  8 +--
>   .../drivers/radeonsi/si_test_dma_perf.c   |  3 +-
>   4 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_compute_blit.c 
> b/src/gallium/drivers/radeonsi/si_compute_blit.c
> index 20e4f591fbb..086793637f0 100644
> --- a/src/gallium/drivers/radeonsi/si_compute_blit.c
> +++ b/src/gallium/drivers/radeonsi/si_compute_blit.c
> @@ -212,22 +212,22 @@ void si_clear_buffer(struct si_context *sctx, struct 
> pipe_resource *dst,
>*/
>   if (clear_value_size > 4 ||
>   (clear_value_size == 4 &&
>offset % 4 == 0 &&
>(size > 32*1024 || sctx->chip_class <= VI))) {
>   si_compute_do_clear_or_copy(sctx, dst, offset, NULL, 0,
>   aligned_size, clear_value,
>   clear_value_size, coher);
>   } else {
>   assert(clear_value_size == 4);
> - si_cp_dma_clear_buffer(sctx, dst, offset,
> -aligned_size, *clear_value, 
> coher,
> + si_cp_dma_clear_buffer(sctx, sctx->gfx_cs, dst, offset,
> +aligned_size, *clear_value, 0, 
> coher,
>  get_cache_policy(sctx, coher, 
> size));
>   }
>
>   offset += aligned_size;
>   size -= aligned_size;
>   }
>
>   /* Handle non-dword alignment. */
>   if (size) {
>   assert(dst);
> diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c 
> b/src/gallium/drivers/radeonsi/si_cp_dma.c
> index 839b31b7fdf..33220d9f0fa 100644
> --- a/src/gallium/drivers/radeonsi/si_cp_dma.c
> +++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
> @@ -47,25 +47,24 @@ static inline unsigned cp_dma_max_byte_count(struct 
> si_context *sctx)
>
>   /* make it aligned for optimal performance */
>   return max & ~(SI_CPDMA_ALIGNMENT - 1);
>   }
>
>
>   /* Emit a CP DMA packet to do a copy from one buffer to another, or to clear
>* a buffer. The size must fit in bits [20:0]. If CP_DMA_CLEAR is set, 
> src_va is a 32-bit
>* clear value.
>*/
> -static void si_emit_cp_dma(struct si_context *sctx, uint64_t dst_va,
> -uint64_t src_va, unsigned size, unsigned flags,
> -enum si_cache_policy cache_policy)
> +static void si_emit_cp_dma(struct si_context *sctx, struct radeon_cmdbuf *cs,
> +uint64_t dst_va, uint64_t src_va, unsigned size,
> +unsigned flags, enum si_cache_policy cache_policy)
>   {
> - struct radeon_cmdbuf *cs = sctx->gfx_cs;
>   uint32_t header = 0, command = 0;
>
>   assert(size <= cp_dma_max_byte_count(sctx));
>   assert(sctx->chip_class != SI || cache_policy == L2_BYPASS);
>
>   if (sctx->chip_class >= GFX9)
>   command |= S_414_BYTE_COUNT_GFX9(size);
>   else
>   command |= S_414_BYTE_COUNT_GFX6(size);
>
> @@ -139,21 +138,21 @@ static void si_emit_cp_dma(struct si_context *sctx, 
> uint64_t dst_va,
>   }
>
>   void si_cp_dma_wait_for_idle(struct si_context *sctx)
>   {
>   /* Issue a dummy DMA that copies zero bytes.
>*
>* The DMA engine will see that there's no work to do and skip this
>* DMA request, however, the CP will see the sync flag and still wait
> 

Re: [Mesa-dev] [PATCH] anv: flush pipeline before query result copies

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 9:52 AM Józef Kucia  wrote:

> On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
>  wrote:
> >
> > Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> > taken into account when copying results.
> >
> > In the particular bug below, the results of the
> > vkCmdCopyQueryPoolResults() command was being overwritten by the
> > preceding vkCmdCopyBuffer() with a same destination buffer. This is
> > because we copy the buffers using the 3D pipeline whereas we copy the
> > query results using the command streamer. Those work in parallel
> > unless synchronized.
> >
> > Signed-off-by: Lionel Landwerlin 
> > Suggested-by: Jason Ekstrand 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
>
> Thanks for the fix.
>
> For what it's worth, we have more test failures on Anv. A lot failures
> are related to clearing multisample array textures. See failures in
> test_clear_render_target_view(), if you are interested.
>

Mind filing a few bugs?  Feel free to bucket them such as "multisample
array texture clears".  How easy is it to run your tests?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: Fix assert in print_intrinsic_instr().

[Józef Kucia]

On Mon, Nov 26, 2018 at 4:23 PM Lionel Landwerlin
 wrote:
>
> On 26/11/2018 15:04, Józef Kucia wrote:
> > Signed-off-by: Józef Kucia 
>
>
> Reviewed-by: Lionel Landwerlin 
>
>

Please push the patch for me. I don't have commit access.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Connor Abbott]

On Thu, Nov 29, 2018 at 4:47 PM Connor Abbott  wrote:
>
> On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  wrote:
> >
> > Can you provide some context for this?  Those rules are already flagged 
> > "inexact" (that's what the ~ means) so they won't apply to anything that's 
> > "precise" or "invariant".
>
> I think the concern is that this isn't allowed in SPIR-V, even without
> exact or invariant. We even go out of our way to do the correct thing
> in the frontend by inserting an "&& a == a" or "|| a != a", but then
> opt_algebraic removes it with another rule and then this rule can flip
> it from ordered to unordered. The spec says that operations don't have
> to produce NaN, but it doesn't say anything on comparisons other than
> the generic "everything must follow IEEE rules" and an entry in the
> table that says "produces correct results." Then again, I can't find
> anything in GLSL allowing these transforms either, so maybe we just
> need to get rid of them.

Sorry... by SPIR-V, I meant Vulkan SPIR-V. Specifically Appendix A of
the Vulkan spec, in the "Precision and Operation of SPIR-V
Instructions" section. There's a section in the GLSL 4.50 spec which
is identical AFAICT.

>
> >
> > On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset  
> > wrote:
> >>
> >> It's correct in GLSL because the behaviour is undefined in
> >> presence of NaNs. But this seems incorrect in Vulkan.
> >>
> >> Signed-off-by: Samuel Pitoiset 
> >> ---
> >>  src/compiler/nir/nir.h| 6 ++
> >>  src/compiler/nir/nir_opt_algebraic.py | 8 
> >>  2 files changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> index db935c8496b..4107c293962 100644
> >> --- a/src/compiler/nir/nir.h
> >> +++ b/src/compiler/nir/nir.h
> >> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
> >> /* Set if nir_lower_wpos_ytransform() should also invert 
> >> gl_PointCoord. */
> >> bool lower_wpos_pntc;
> >>
> >> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
> >> +* In presence of NaNs, this is correct in GLSL because the 
> >> behaviour is
> >> +* undefined. In Vulkan, doing these transformations is incorrect.
> >> +*/
> >> +   bool exact_float_comparisons;
> >> +
> >> /**
> >>  * Should nir_lower_io() create load_interpolated_input intrinsics?
> >>  *
> >> diff --git a/src/compiler/nir/nir_opt_algebraic.py 
> >> b/src/compiler/nir/nir_opt_algebraic.py
> >> index f2a7be0c403..3750874407b 100644
> >> --- a/src/compiler/nir/nir_opt_algebraic.py
> >> +++ b/src/compiler/nir/nir_opt_algebraic.py
> >> @@ -154,10 +154,10 @@ optimizations = [
> >> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
> >>
> >> # Comparison simplifications
> >> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
> >> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
> >> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
> >> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
> >> +   (('~inot', ('flt', a, b)), ('fge', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('fge', a, b)), ('flt', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('feq', a, b)), ('fne', a, b), 
> >> '!options->exact_float_comparisons'),
> >> +   (('~inot', ('fne', a, b)), ('feq', a, b), 
> >> '!options->exact_float_comparisons'),
> >
> >
> > The feq/fne one is actually completely safe.  fne is defined to be !feq 
> > even when NaN is considered.
> >
> > --Jasoan
> >
> >>
> >> (('inot', ('ilt', a, b)), ('ige', a, b)),
> >> (('inot', ('ult', a, b)), ('uge', a, b)),
> >> (('inot', ('ige', a, b)), ('ilt', a, b)),
> >> --
> >> 2.19.2
> >>
> >> ___
> >> 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] anv: flush pipeline before query result copies

[Józef Kucia]

On Thu, Nov 29, 2018 at 4:43 PM Lionel Landwerlin
 wrote:
>
> Pipeline barriers inserted through vkCmdPipelineBarrier() should be
> taken into account when copying results.
>
> In the particular bug below, the results of the
> vkCmdCopyQueryPoolResults() command was being overwritten by the
> preceding vkCmdCopyBuffer() with a same destination buffer. This is
> because we copy the buffers using the 3D pipeline whereas we copy the
> query results using the command streamer. Those work in parallel
> unless synchronized.
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Jason Ekstrand 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894

Thanks for the fix.

For what it's worth, we have more test failures on Anv. A lot failures
are related to clearing multisample array textures. See failures in
test_clear_render_target_view(), if you are interested.

Thanks,
Józef
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH mesa 1/2] docs: add meson cross compilation instructions

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
The 32-bit linux one is tested, but the other two are "I think this
should be right".
---
 docs/meson.html | 83 +
 1 file changed, 83 insertions(+)

diff --git a/docs/meson.html b/docs/meson.html
index 68f80d6ac42d6f48d359..baa1f0f327e99d86e9a5 100644
--- a/docs/meson.html
+++ b/docs/meson.html
@@ -16,6 +16,11 @@ The Mesa 3D Graphics Library
 
 Compilation and Installation using Meson
 
+
+  Basic Usage
+  Cross-compilation and 32-bit builds
+
+
 1. Basic Usage
 
 The Meson build system is generally considered stable and ready
@@ -190,6 +195,84 @@ 1. Basic Usage
 
 
 
+2. Cross-compilation and 32-bit builds
+
+https://mesonbuild.com/Cross-compilation.html;>Meson supports
+cross-compilation by specifying a number of binary paths and
+settings in a file and passing this file to meson or
+meson configure with the --cross-file
+parameter.
+
+This file can live at any location, but you can use the bare filename
+(without the folder path) if you put it in $XDG_DATA_HOME/meson/cross or
+~/.local/share/meson/cross
+
+Below are a few example of cross files, but keep in mind that you
+will likely have to alter them for your system.
+
+
+32-bit build on x86 linux:
+
+[binaries]
+c = '/usr/bin/gcc'
+cpp = '/usr/bin/g++'
+ar = '/usr/bin/ar'
+strip = '/usr/bin/strip'
+pkgconfig = '/usr/bin/pkg-config-32'
+llvm-config = '/usr/bin/llvm-config'
+
+[properties]
+c_args = ['-m32']
+c_link_args = ['-m32']
+cpp_args = ['-m32']
+cpp_link_args = ['-m32']
+
+[host_machine]
+system = 'linux'
+cpu_family = 'x86'
+cpu = 'i686'
+endian = 'little'
+
+
+
+
+64-bit build on ARM linux:
+
+[binaries]
+c = '/usr/bin/aarch64-linux-gnu-gcc'
+cpp = '/usr/bin/aarch64-linux-gnu-g++'
+ar = '/usr/bin/aarch64-linux-gnu-ar'
+strip = '/usr/bin/aarch64-linux-gnu-strip'
+pkgconfig = '/usr/bin/aarch64-linux-gnu-pkg-config'
+exe_wrapper = '/usr/bin/qemu-aarch64-static'
+
+[host_machine]
+system = 'linux'
+cpu_family = 'arm'
+cpu = 'aarch64'
+endian = 'little'
+
+
+
+
+64-bit build on x86 windows:
+
+[binaries]
+c = '/usr/bin/x86_64-w64-mingw32-gcc'
+cpp = '/usr/bin/x86_64-w64-mingw32-g++'
+ar = '/usr/bin/x86_64-w64-mingw32-ar'
+strip = '/usr/bin/x86_64-w64-mingw32-strip'
+pkgconfig = '/usr/bin/x86_64-w64-mingw32-pkg-config'
+exe_wrapper = 'wine'
+
+[host_machine]
+system = 'windows'
+cpu_family = 'x86_64'
+cpu = 'i686'
+endian = 'little'
+
+
+
 
 
 
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH mesa 2/2] docs: format code blocks a bit nicely

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 docs/mesa.css | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/docs/mesa.css b/docs/mesa.css
index 7ab8152b040e6f469711..cfdcc94a06759befd568 100644
--- a/docs/mesa.css
+++ b/docs/mesa.css
@@ -29,6 +29,9 @@ pre {
/*font-family: monospace;*/
font-size: 10pt;
/*color: black;*/
+   background-color: #eee;
+   margin-left: 2em;
+   padding: .5em;
 }
 
 iframe {
-- 
Cheers,
  Eric

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

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Connor Abbott]

On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  wrote:
>
> Can you provide some context for this?  Those rules are already flagged 
> "inexact" (that's what the ~ means) so they won't apply to anything that's 
> "precise" or "invariant".

I think the concern is that this isn't allowed in SPIR-V, even without
exact or invariant. We even go out of our way to do the correct thing
in the frontend by inserting an "&& a == a" or "|| a != a", but then
opt_algebraic removes it with another rule and then this rule can flip
it from ordered to unordered. The spec says that operations don't have
to produce NaN, but it doesn't say anything on comparisons other than
the generic "everything must follow IEEE rules" and an entry in the
table that says "produces correct results." Then again, I can't find
anything in GLSL allowing these transforms either, so maybe we just
need to get rid of them.

>
> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset  
> wrote:
>>
>> It's correct in GLSL because the behaviour is undefined in
>> presence of NaNs. But this seems incorrect in Vulkan.
>>
>> Signed-off-by: Samuel Pitoiset 
>> ---
>>  src/compiler/nir/nir.h| 6 ++
>>  src/compiler/nir/nir_opt_algebraic.py | 8 
>>  2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index db935c8496b..4107c293962 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
>> /* Set if nir_lower_wpos_ytransform() should also invert gl_PointCoord. 
>> */
>> bool lower_wpos_pntc;
>>
>> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
>> +* In presence of NaNs, this is correct in GLSL because the 
>> behaviour is
>> +* undefined. In Vulkan, doing these transformations is incorrect.
>> +*/
>> +   bool exact_float_comparisons;
>> +
>> /**
>>  * Should nir_lower_io() create load_interpolated_input intrinsics?
>>  *
>> diff --git a/src/compiler/nir/nir_opt_algebraic.py 
>> b/src/compiler/nir/nir_opt_algebraic.py
>> index f2a7be0c403..3750874407b 100644
>> --- a/src/compiler/nir/nir_opt_algebraic.py
>> +++ b/src/compiler/nir/nir_opt_algebraic.py
>> @@ -154,10 +154,10 @@ optimizations = [
>> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
>>
>> # Comparison simplifications
>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
>> +   (('~inot', ('flt', a, b)), ('fge', a, b), 
>> '!options->exact_float_comparisons'),
>> +   (('~inot', ('fge', a, b)), ('flt', a, b), 
>> '!options->exact_float_comparisons'),
>> +   (('~inot', ('feq', a, b)), ('fne', a, b), 
>> '!options->exact_float_comparisons'),
>> +   (('~inot', ('fne', a, b)), ('feq', a, b), 
>> '!options->exact_float_comparisons'),
>
>
> The feq/fne one is actually completely safe.  fne is defined to be !feq even 
> when NaN is considered.
>
> --Jasoan
>
>>
>> (('inot', ('ilt', a, b)), ('ige', a, b)),
>> (('inot', ('ult', a, b)), ('uge', a, b)),
>> (('inot', ('ige', a, b)), ('ilt', a, b)),
>> --
>> 2.19.2
>>
>> ___
>> 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] [PATCH] anv: flush pipeline before query result copies

[Lionel Landwerlin]

Pipeline barriers inserted through vkCmdPipelineBarrier() should be
taken into account when copying results.

In the particular bug below, the results of the
vkCmdCopyQueryPoolResults() command was being overwritten by the
preceding vkCmdCopyBuffer() with a same destination buffer. This is
because we copy the buffers using the 3D pipeline whereas we copy the
query results using the command streamer. Those work in parallel
unless synchronized.

Signed-off-by: Lionel Landwerlin 
Suggested-by: Jason Ekstrand 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108894
Cc: mesa-sta...@lists.freedesktop.org
---
 src/intel/vulkan/genX_query.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c
index ce8757f2643..edf9962179a 100644
--- a/src/intel/vulkan/genX_query.c
+++ b/src/intel/vulkan/genX_query.c
@@ -730,10 +730,9 @@ void genX(CmdCopyQueryPoolResults)(
ANV_FROM_HANDLE(anv_buffer, buffer, destBuffer);
 
if (flags & VK_QUERY_RESULT_WAIT_BIT) {
-  anv_batch_emit(_buffer->batch, GENX(PIPE_CONTROL), pc) {
- pc.CommandStreamerStallEnable = true;
- pc.StallAtPixelScoreboard = true;
-  }
+  if (cmd_buffer->state.pending_pipe_bits & ANV_PIPE_FLUSH_BITS)
+ cmd_buffer->state.pending_pipe_bits |= ANV_PIPE_CS_STALL_BIT;
+  genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
}
 
struct anv_address dest_addr = anv_address_add(buffer->address, destOffset);
-- 
2.20.0.rc1

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

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Jason Ekstrand]

Can you provide some context for this?  Those rules are already flagged
"inexact" (that's what the ~ means) so they won't apply to anything that's
"precise" or "invariant".

On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset 
wrote:

> It's correct in GLSL because the behaviour is undefined in
> presence of NaNs. But this seems incorrect in Vulkan.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/compiler/nir/nir.h| 6 ++
>  src/compiler/nir/nir_opt_algebraic.py | 8 
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index db935c8496b..4107c293962 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
> /* Set if nir_lower_wpos_ytransform() should also invert
> gl_PointCoord. */
> bool lower_wpos_pntc;
>
> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
> +* In presence of NaNs, this is correct in GLSL because the
> behaviour is
> +* undefined. In Vulkan, doing these transformations is incorrect.
> +*/
> +   bool exact_float_comparisons;
> +
> /**
>  * Should nir_lower_io() create load_interpolated_input intrinsics?
>  *
> diff --git a/src/compiler/nir/nir_opt_algebraic.py
> b/src/compiler/nir/nir_opt_algebraic.py
> index f2a7be0c403..3750874407b 100644
> --- a/src/compiler/nir/nir_opt_algebraic.py
> +++ b/src/compiler/nir/nir_opt_algebraic.py
> @@ -154,10 +154,10 @@ optimizations = [
> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
>
> # Comparison simplifications
> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
> +   (('~inot', ('flt', a, b)), ('fge', a, b),
> '!options->exact_float_comparisons'),
> +   (('~inot', ('fge', a, b)), ('flt', a, b),
> '!options->exact_float_comparisons'),
> +   (('~inot', ('feq', a, b)), ('fne', a, b),
> '!options->exact_float_comparisons'),
> +   (('~inot', ('fne', a, b)), ('feq', a, b),
> '!options->exact_float_comparisons'),
>

The feq/fne one is actually completely safe.  fne is defined to be !feq
even when NaN is considered.

--Jasoan


> (('inot', ('ilt', a, b)), ('ige', a, b)),
> (('inot', ('ult', a, b)), ('uge', a, b)),
> (('inot', ('ige', a, b)), ('ilt', a, b)),
> --
> 2.19.2
>
> ___
> 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] docs: Document optional GitLab code review process

[Jason Ekstrand]

On Thu, Nov 29, 2018 at 3:11 AM Erik Faye-Lund 
wrote:

> On Tue, 2018-11-27 at 17:13 -0800, Jordan Justen wrote:
> > This documents a mechanism for using GitLab Merge Requests as an
> > optional, secondary way to get code reviews for patchsets.
> >
> > We still require all patches to be emailed.
> >
> > Aside from the potential usage for code review comments, it might
> > also
> > help reviewers to find unmerged patchsets which need review.
> > (Assuming
> > it doesn't fall into the same fate of patchwork with hundreds of open
> > MRs.)
> >
> > Signed-off-by: Jordan Justen 
> > Cc: Jason Ekstrand 
>
> In lack of a better thread to discuss this, I'm going to hijack this
> one slightly. I hope I'm not too disturbing.
>
> For the better part of the last 4 years, I've been working on a large-
> ish (commercial, but open source) project that used GitHub PRs for
> contributions. It's a great way of working, for all the reasons we've
> already been through:
> - Structured reviews
> - Easy to get an overview what's happening
> - Prevent most bad merges due to CI
>
> But there's a few down-sides as well, compared to mailing list
> discussions, and these are the reasons I wanted to speak up. I'm a
> strong supporter for MRs, by the way. I just want these issues talked
> about. Also note that my experience here comes from GitHub, and not
> specifically from GitLab, but I suspect at least some of them apply
> here as well:
>
> 1. There's usually no reasonable way of reviewing commit messages. You
> can add general comments on the development history, but it gets
> awkward to navigate between tabs to copy something to quote and stuff.
> In e-mail based reviews, you just click reply.
>
> 2. It's often difficult to see comments on intermediate commits. At
> least on GitHub, intermediate changes gets collapsed as "out-of-date",
> because GitHub doesn't seem to care about clean development history.
> This tends to
>

Both of the above are issues that various freedesktop.org projects have
raised with the GitLab community and are things they are actively
improving.  It's not amazing yet, but there is active improvement for
no-merge fast-forward commit series review.  In particular, I believe 2 is
more-or-less sorted now.


> 3. The MR list becomes something that needs to be maintained. Sometimes
> there's long-lived MRs that takes months to get merged, or just simply
> stall... and they all start piling up. That's fine in a mailing list,
> but it quickly gets cumbersome in a MR-based work-flow, as old MRs take
> up space in UIs, and makes it harder to see what's going on. So at some
> point, stale MRs should be closed IMO. This is going to need someone to
> either do it, or to maintain some bot to do it. And some developers
> tends to get angry when you close a MR after three months of inactivity
> just because nobody wanted to review it.
>

I'm not too concerned with this.  We can easily write a bot or something.

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

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Eric Engestrom]

On Wednesday, 2018-11-28 13:36:29 -0800, Eric Anholt wrote:
> Jordan Justen  writes:
> 
> > This documents a mechanism for using GitLab Merge Requests as an
> > optional, secondary way to get code reviews for patchsets.
> >
> > We still require all patches to be emailed.
> >
> > Aside from the potential usage for code review comments, it might also
> > help reviewers to find unmerged patchsets which need review. (Assuming
> > it doesn't fall into the same fate of patchwork with hundreds of open
> > MRs.)
> >
> > Signed-off-by: Jordan Justen 
> > Cc: Jason Ekstrand 
> > ---
> >  docs/submittingpatches.html | 25 +
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > index 5d8ba443191..852f28c198a 100644
> > --- a/docs/submittingpatches.html
> > +++ b/docs/submittingpatches.html
> > @@ -24,6 +24,7 @@
> >  Testing Patches
> >  Mailing Patches
> >  Reviewing Patches
> > +GitLab Merge Requests
> >  Nominating a commit for a stable branch
> >  Criteria for accepting patches to the stable 
> > branch
> >  Sending backports for the stable branch
> > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> > committed, as long
> >  as the issues are resolved first.
> >  
> >  
> > +GitLab Merge Requests
> > +
> > +
> > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> > +  Requests (MR) can be used as an optional, secondary method of
> > +  obtaining code review for patches.
> > +
> > +
> > +
> > +  All patches should be submitted using email as well
> 
> Like others, I disagree and think this will lead to confusion.  Let
> people send to one or the other.
> 
> > +  Consider including a link to the MR in your email based
> > +cover-letter
> > +  Address code review from both email and the MR
> > +  Add appropriate labels to your MR. For example:
> > +
> > +  Mesa changes affect all drivers: mesa
> > +  Hardware vendor specific code: amd, intel, nvidia, etc
> > +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
> > radv, vc4, etc
> > +  Other tag examples: gallium, util
> > +
> > +  Never use the merge button on the GitLab page to push patches
> 
> Why "never use the merge button"?  We've been using rebase+merge in
> xserver and it's *awesome* and has greatly increased the review rate.

Because GitLab doesn't (yet) support tracking 'reviewed' status in
the commit message, which is something I think is important to track for
various reasons. If those are manually added to the commit messages
before the MR is merged, then there's no issue there.

We could also have a check that an MR can't be merged until all its
commits contain the string "Reviewed-by:", which might be a better
solution?

("yet" because there's some upstream discussion on this, but it
hasn't materialized into an MR yet.)

> 
> > +  Add Reviewed-by tags to your commits and push using git
> > +  Close your MR when your patches get pushed!
> > +
> >  
> >  Nominating a commit for a stable branch
> 
> Overall:
> 
> I can't wait to have a full MR process.  What if we just *never* merged
> code that broke the build or introduced warnings, because the MR process
> ensured it?  How much time would we all save?  How much less training
> would we need to do on new contributors?

Agreed on all counts, and I believe we'll get there soon ;)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Samuel Pitoiset]

It's correct in GLSL because the behaviour is undefined in
presence of NaNs. But this seems incorrect in Vulkan.

Signed-off-by: Samuel Pitoiset 
---
 src/compiler/nir/nir.h| 6 ++
 src/compiler/nir/nir_opt_algebraic.py | 8 
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index db935c8496b..4107c293962 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
/* Set if nir_lower_wpos_ytransform() should also invert gl_PointCoord. */
bool lower_wpos_pntc;
 
+   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
+* In presence of NaNs, this is correct in GLSL because the behaviour is
+* undefined. In Vulkan, doing these transformations is incorrect.
+*/
+   bool exact_float_comparisons;
+
/**
 * Should nir_lower_io() create load_interpolated_input intrinsics?
 *
diff --git a/src/compiler/nir/nir_opt_algebraic.py 
b/src/compiler/nir/nir_opt_algebraic.py
index f2a7be0c403..3750874407b 100644
--- a/src/compiler/nir/nir_opt_algebraic.py
+++ b/src/compiler/nir/nir_opt_algebraic.py
@@ -154,10 +154,10 @@ optimizations = [
(('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
 
# Comparison simplifications
-   (('~inot', ('flt', a, b)), ('fge', a, b)),
-   (('~inot', ('fge', a, b)), ('flt', a, b)),
-   (('~inot', ('feq', a, b)), ('fne', a, b)),
-   (('~inot', ('fne', a, b)), ('feq', a, b)),
+   (('~inot', ('flt', a, b)), ('fge', a, b), 
'!options->exact_float_comparisons'),
+   (('~inot', ('fge', a, b)), ('flt', a, b), 
'!options->exact_float_comparisons'),
+   (('~inot', ('feq', a, b)), ('fne', a, b), 
'!options->exact_float_comparisons'),
+   (('~inot', ('fne', a, b)), ('feq', a, b), 
'!options->exact_float_comparisons'),
(('inot', ('ilt', a, b)), ('ige', a, b)),
(('inot', ('ult', a, b)), ('uge', a, b)),
(('inot', ('ige', a, b)), ('ilt', a, b)),
-- 
2.19.2

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

[Mesa-dev] [PATCH 2/2] radv: do not lower float comparisons because they have to be exact

[Samuel Pitoiset]

This fixes two test failures in the VKD3D testsuite.
(ie. in test_shader_instructions().

26260 shaders in 13493 tests
Totals:
SGPRS: 1203796 -> 1203948 (0.01 %)
VGPRS: 844968 -> 845052 (0.01 %)
Spilled SGPRs: 21844 -> 21821 (-0.11 %)
Spilled VGPRs: 122 -> 122 (0.00 %)
Code Size: 44707992 -> 44635672 (-0.16 %) bytes
Max Waves: 219539 -> 219529 (-0.00 %)

Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_shader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 456c462a230..870105fa85c 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -71,7 +71,8 @@ static const struct nir_shader_compiler_options nir_options = 
{
.lower_extract_word = true,
.lower_ffma = true,
.lower_fpow = true,
-   .max_unroll_iterations = 32
+   .max_unroll_iterations = 32,
+   .exact_float_comparisons = true,
 };
 
 VkResult radv_CreateShaderModule(
-- 
2.19.2

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

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Eric Engestrom]

On Thursday, 2018-11-29 10:11:22 +0100, Erik Faye-Lund wrote:
> On Tue, 2018-11-27 at 17:13 -0800, Jordan Justen wrote:
> > This documents a mechanism for using GitLab Merge Requests as an
> > optional, secondary way to get code reviews for patchsets.
> > 
> > We still require all patches to be emailed.
> > 
> > Aside from the potential usage for code review comments, it might
> > also
> > help reviewers to find unmerged patchsets which need review.
> > (Assuming
> > it doesn't fall into the same fate of patchwork with hundreds of open
> > MRs.)
> > 
> > Signed-off-by: Jordan Justen 
> > Cc: Jason Ekstrand 
> 
> In lack of a better thread to discuss this, I'm going to hijack this
> one slightly. I hope I'm not too disturbing.
> 
> For the better part of the last 4 years, I've been working on a large-
> ish (commercial, but open source) project that used GitHub PRs for
> contributions. It's a great way of working, for all the reasons we've
> already been through:
> - Structured reviews
> - Easy to get an overview what's happening
> - Prevent most bad merges due to CI
> 
> But there's a few down-sides as well, compared to mailing list
> discussions, and these are the reasons I wanted to speak up. I'm a
> strong supporter for MRs, by the way. I just want these issues talked
> about. Also note that my experience here comes from GitHub, and not
> specifically from GitLab, but I suspect at least some of them apply
> here as well:
> 
> 1. There's usually no reasonable way of reviewing commit messages. You
> can add general comments on the development history, but it gets
> awkward to navigate between tabs to copy something to quote and stuff.
> In e-mail based reviews, you just click reply.

GitLab recently changed the way commit messages are shown, which should
help with this. The way it used to show them (and GitHub still does)
basically means that the individual commits have no meaning and this was
really annoying, but I think GitLab fixed this now.

> 
> 2. It's often difficult to see comments on intermediate commits. At
> least on GitHub, intermediate changes gets collapsed as "out-of-date",
> because GitHub doesn't seem to care about clean development history.
> This tends to 

(unfinished sentence?)

I don't really have experience with that on GitLab, so I can't comment.

> 
> 3. The MR list becomes something that needs to be maintained. Sometimes
> there's long-lived MRs that takes months to get merged, or just simply
> stall... and they all start piling up. That's fine in a mailing list,
> but it quickly gets cumbersome in a MR-based work-flow, as old MRs take
> up space in UIs, and makes it harder to see what's going on. So at some
> point, stale MRs should be closed IMO. This is going to need someone to
> either do it, or to maintain some bot to do it. And some developers
> tends to get angry when you close a MR after three months of inactivity
> just because nobody wanted to review it.

Agreed, I forgot to mention earlier but you right, we need a bot that
closes MRs after 3 months with a message along the lines of:
> This has been auto-closed due to inactivity; if you think it should
> still be opened, please re-open with a comment.

This should get rid of most of the forgotten MRs.

> 
> Out of these, #1 and #2 are my biggest nusances with it, really. The
> PR/MR workflow tends to be all about the end-result, and not so much
> about the intermediate steps or the "story" they tell. And the UIs
> seems to reward such development rather than "the story matters"-
> development.
> 
> I'm a bit worried that we'd start gradually changing how we developed;
> I saw this happen to some degree in my past experience. But at the same
> time, I do think that the benefits greatly outweigh the disadvantages.

Agreed on all of the above :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 108900] GPU hangs with GfxBench v5 Aztec Ruins Vulkan test

[bugzilla-daemon]

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

Bug ID: 108900
   Summary: GPU hangs with GfxBench v5 Aztec Ruins Vulkan test
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Vulkan/radeon
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: eero.t.tammi...@intel.com
QA Contact: mesa-dev@lists.freedesktop.org

Setup:
- FullHD monitor (through HDMI KVM)
- HadesCanyon KBL i7-8809G ([AMD/ATI] Vega [Radeon RX Vega M] (rev c0))
- Ubuntu 18.04
- drm-tip git kernel v4.20-rc4 (i.e. kernel.org v4.20-rc4 kernel + latest drm
code from yesterday)
- Mesa git (c120dbfe4d)
- X server git version
- Proprietary GfxBench v5-GOLD2:  http://gfxbench.com

Test-case:
* bin/testfw_app --gfx vulkan --gl_api vulkan --width 1920 --height 1080
--fullscreen 1 --test_id vulkan_5_normal

Expected outcome:
* Works fine like the Aztec Ruins GL version and Sacha Willems' Vulkan tests,
no GPU hangs

Actual outcome:
* Right after test starts, following in dmesg:
-
[ 3057.480868] amdgpu :01:00.0: GPU fault detected: 146 0x0fa0880c for
process testfw_app pid 2995 thread testfw_app pid 2997
[ 3057.480870] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x001001F4
[ 3057.480871] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x0C08800C
[ 3057.480873] amdgpu :01:00.0: VM fault (0x0c, vmid 6, pasid 32772) at
page 1049076, read from 'TC4' (0x54433400) (136)
[ 3057.480879] amdgpu :01:00.0: GPU fault detected: 146 0x0fa0840c for
process testfw_app pid 2995 thread testfw_app pid 2997
[ 3057.480880] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x001001FD
[ 3057.480881] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x0C08400C
[ 3057.480883] amdgpu :01:00.0: VM fault (0x0c, vmid 6, pasid 32772) at
page 1049085, read from 'TC5' (0x54433500) (132)
[ 3057.480944] amdgpu :01:00.0: GPU fault detected: 146 0x0fa9080c for
process testfw_app pid 2995 thread testfw_app pid 2997
[ 3057.480945] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x
[ 3057.480946] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x0C18802C
[ 3057.480947] amdgpu :01:00.0: VM fault (0x2c, vmid 6, pasid 32772) at
page 0, read from 'TC0' (0x54433000) (392)
[ 3067.564630] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout,
signaled seq=53811, emitted seq=53814
[ 3067.564633] [drm] GPU recovery disabled.
-

After this, no other GPU operations seem to work properly.  There are also
other things that don't work properly in automated testing at this point, but
I'm not sure whether they're related.

No idea whether this is a regression as I checked it only now.  There are some 
issues with this particular test also on Intel (see e.g. bug 104634, bug
105276), so the problem could be in common code.  No idea whether this is
related to GL bug 108898 on same device.

-- 
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] [ANNOUNCE] mesa 18.3.0-rc5

[Emil Velikov]

The fifth release candidate for Mesa 18.3.0 is now available.

As per the issue tracker [1] we have one outstanding bug blocking the
release.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=108530


Bas Nieuwenhuizen (3):
  radv: Fix opaque metadata descriptor last layer.
  radv: Clamp gfx9 image view extents to the allocated image extents.
  radv: Align large buffers to the fragment size.

Emil Velikov (4):
  egl/wayland: bail out when drmGetMagic fails
  egl/wayland: plug memory leak in drm_handle_device()
  cherry-ignore: egl/wayland: rather obvious build fix
  Update version to 18.3.0-rc5

Eric Engestrom (4):
  glapi: add missing visibility args
  anv: correctly use vulkan 1.0 by default
  wsi/display: fix mem leak when freeing swapchains
  vulkan/wsi: fix s/,/;/ typo

Erik Faye-Lund (5):
  mesa/main: remove bogus error for zero-sized images
  mesa/main: factor out tex-image error-checking
  mesa/main: factor out common error-checking
  mesa/main: check cube-completeness in common code
  mesa/main: fix incorrect depth-error

Gert Wollny (1):
  glsl: free or reuse memory allocated for TF varying

Jason Ekstrand (1):
  anv: Put robust buffer access in the pipeline hash

Marek Olšák (2):
  winsys/amdgpu: fix a buffer leak in amdgpu_bo_from_handle
  winsys/amdgpu: fix a device handle leak in amdgpu_winsys_create

git tag: mesa-18.3.0-rc5

https://mesa.freedesktop.org/archive/mesa-18.3.0-rc5.tar.gz
MD5:  f7bd91c48522dcd3cb3da0aa286e6cd9  mesa-18.3.0-rc5.tar.gz
SHA1: c9260058becddc16183e767b28c00f95a250a735  mesa-18.3.0-rc5.tar.gz
SHA256: 596b61a1b66db3e0c95cdd496b32dd32612b1c88491aafe2331e9620cb0a6b72  
mesa-18.3.0-rc5.tar.gz
SHA512: 
a569cb426fc6d178f809248f7c0bf0bb2996bc2ebfdc59a0ac6156f6e18cb8de6fbd74c247a571d7284a38c1b457c78e2867ee979ab83795940e0eadfc9e1800
  mesa-18.3.0-rc5.tar.gz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.3.0-rc5.tar.gz.sig

https://mesa.freedesktop.org/archive/mesa-18.3.0-rc5.tar.xz
MD5:  a23c83b2037c809828abe434d08a867e  mesa-18.3.0-rc5.tar.xz
SHA1: 51dfcbdafe2881b58113e9411c3e20678cf75f10  mesa-18.3.0-rc5.tar.xz
SHA256: d8a2c19719a7b14ae3725ef33d11d640ebfff5afb793e92c0a5740f0a2dd548a  
mesa-18.3.0-rc5.tar.xz
SHA512: 
989eb74030ca5bd0fccd955be8370777d6935ff02302c5a8c01aa4b5b02f357c2191afbdbaace3892cb3772249e16667b5e77bd5326300667ec94b0dbb8530aa
  mesa-18.3.0-rc5.tar.xz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.3.0-rc5.tar.xz.sig




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

[Mesa-dev] [PATCH mesa] meson: skip asm check when asm is disabled

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 4ee40503d044a0985c89..2d300d0421980cba7866 100644
--- a/meson.build
+++ b/meson.build
@@ -917,7 +917,7 @@ endif
 # case of cross compiling where we can use asm, and that's x86_64 -> x86 when
 # host OS == build OS, since in that case the build machine can run the host's
 # binaries.
-if meson.is_cross_build() 
+if with_asm and meson.is_cross_build()
   if build_machine.system() != host_machine.system()
 # TODO: It may be possible to do this with an exe_wrapper (like wine).
 message('Cross compiling from one OS to another, disabling assembly.')
-- 
Cheers,
  Eric

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

[Mesa-dev] [Bug 105132] glxSwapBuffers has no effect

[bugzilla-daemon]

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

Yuxuan Shui  changed:

   What|Removed |Added

 Blocks||108651


Referenced Bugs:

https://bugs.freedesktop.org/show_bug.cgi?id=108651
[Bug 108651] Screen flickering when using compositor with OpenGL backend
-- 
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 108705] GLX with DRI2 doesn't work properly when xcb is owning the event queue

[bugzilla-daemon]

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

Yuxuan Shui  changed:

   What|Removed |Added

 Blocks||108651


Referenced Bugs:

https://bugs.freedesktop.org/show_bug.cgi?id=108651
[Bug 108651] Screen flickering when using compositor with OpenGL backend
-- 
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

Re: [Mesa-dev] [PATCH 2/3] mesa: expose EXT_texture_compression_bptc in GLES

[Erik Faye-Lund]

On Wed, 2018-11-28 at 20:22 -0500, Marek Olšák wrote:
> From: Marek Olšák 
> 
> tested by piglit.
> ---
>  docs/relnotes/19.0.0.html|  1 +
>  src/mesa/main/extensions_table.h |  1 +
>  src/mesa/main/glformats.c| 13 +++--
>  src/mesa/main/texcompress.c  |  9 +
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index 50cdda6cbc7..d10bd2cf720 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version
> depending on each driver.
>  SHA256 checksums
>  
>  TBD.
>  
>  
>  
>  New features
>  
>  
>  GL_EXT_shader_implicit_conversions on all drivers (ES
> extension).
> +GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES
> extension).
>  GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES
> extension).
>  GL_EXT_texture_view on drivers supporting texture views (ES
> extension).
>  GL_OES_texture_view on drivers supporting texture views (ES
> extension).
>  
>  
>  Bug fixes
>  
>  
>  TBD
>  
> diff --git a/src/mesa/main/extensions_table.h
> b/src/mesa/main/extensions_table.h
> index dd7a4d45079..45ee7675ab2 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -269,20 +269,21 @@ EXT(EXT_shadow_funcs,
> ARB_shadow
>  EXT(EXT_stencil_two_side,
> EXT_stencil_two_side   , GLL,  x ,  x ,  x , 2001)
>  EXT(EXT_stencil_wrap,
> dummy_true , GLL,  x ,  x ,  x , 2002)
>  EXT(EXT_subtexture  ,
> dummy_true , GLL,  x ,  x ,  x , 1995)
>  EXT(EXT_tessellation_point_size ,
> ARB_tessellation_shader,  x ,  x ,  x ,  31, 2013)
>  EXT(EXT_tessellation_shader ,
> ARB_tessellation_shader,  x ,  x ,  x ,  31, 2013)
>  EXT(EXT_texture ,
> dummy_true , GLL,  x ,  x ,  x , 1996)
>  EXT(EXT_texture3D   ,
> dummy_true , GLL,  x ,  x ,  x , 1996)
>  EXT(EXT_texture_array   ,
> EXT_texture_array  , GLL, GLC,  x ,  x , 2006)
>  EXT(EXT_texture_border_clamp,
> ARB_texture_border_clamp   ,  x ,  x ,  x , ES2, 2014)
>  EXT(EXT_texture_buffer  ,
> OES_texture_buffer ,  x ,  x ,  x ,  31, 2014)
> +EXT(EXT_texture_compression_bptc,
> ARB_texture_compression_bptc   ,  x ,  x ,  x ,  30, 2017)
>  EXT(EXT_texture_compression_dxt1,
> ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2, 2004)
>  EXT(EXT_texture_compression_latc,
> EXT_texture_compression_latc   , GLL,  x ,  x ,  x , 2006)
>  EXT(EXT_texture_compression_rgtc,
> ARB_texture_compression_rgtc   , GLL, GLC,  x ,  30, 2004)
>  EXT(EXT_texture_compression_s3tc,
> EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2, 2000)
>  EXT(EXT_texture_cube_map,
> ARB_texture_cube_map   , GLL,  x ,  x ,  x , 2001)
>  EXT(EXT_texture_cube_map_array  ,
> OES_texture_cube_map_array ,  x ,  x ,  x ,  31, 2014)
>  EXT(EXT_texture_edge_clamp  ,
> dummy_true , GLL,  x ,  x ,  x , 1997)
>  EXT(EXT_texture_env_add ,
> dummy_true , GLL,  x ,  x ,  x , 1999)
>  EXT(EXT_texture_env_combine ,
> dummy_true , GLL,  x ,  x ,  x , 2000)
>  EXT(EXT_texture_env_dot3,
> EXT_texture_env_dot3   , GLL,  x ,  x ,  x , 2000)
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index bacaa9519ca..98ae5d93234 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1389,22 +1389,21 @@ _mesa_is_compressed_format(const struct
> gl_context *ctx, GLenum format)
>return ctx->Extensions.ARB_texture_compression_rgtc;
> case MESA_FORMAT_LAYOUT_LATC:
>return ctx->API == API_OPENGL_COMPAT
>   && ctx->Extensions.EXT_texture_compression_latc;
> case MESA_FORMAT_LAYOUT_ETC1:
>return _mesa_is_gles(ctx)
>   && ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
> case MESA_FORMAT_LAYOUT_ETC2:
>return _mesa_is_gles3(ctx) || ctx-
> >Extensions.ARB_ES3_compatibility;
> case MESA_FORMAT_LAYOUT_BPTC:
> -  return _mesa_is_desktop_gl(ctx) &&
> - ctx->Extensions.ARB_texture_compression_bptc;
> +  return ctx->Extensions.ARB_texture_compression_bptc;

This patch also enables things for GLES 1.x, similar to the previous
patch.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

Re: [Mesa-dev] [PATCH 1/3] mesa: expose EXT_texture_compression_rgtc on GLES

[Erik Faye-Lund]

On Wed, 2018-11-28 at 20:22 -0500, Marek Olšák wrote:
> From: Marek Olšák 
> 
> The spec was modified to support GLES. Tested by piglit.
> ---
>  docs/relnotes/19.0.0.html|  1 +
>  src/mesa/main/extensions_table.h |  2 +-
>  src/mesa/main/glformats.c| 19 +--
>  src/mesa/main/texcompress.c  |  9 +
>  4 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index 920cf803f5d..50cdda6cbc7 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version
> depending on each driver.
>  SHA256 checksums
>  
>  TBD.
>  
>  
>  
>  New features
>  
>  
>  GL_EXT_shader_implicit_conversions on all drivers (ES
> extension).
> +GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES
> extension).
>  GL_EXT_texture_view on drivers supporting texture views (ES
> extension).
>  GL_OES_texture_view on drivers supporting texture views (ES
> extension).
>  
>  
>  Bug fixes
>  
>  
>  TBD
>  
>  
> diff --git a/src/mesa/main/extensions_table.h
> b/src/mesa/main/extensions_table.h
> index 4f2707b65a4..dd7a4d45079 100644
> --- a/src/mesa/main/extensions_table.h
> +++ b/src/mesa/main/extensions_table.h
> @@ -271,21 +271,21 @@ EXT(EXT_stencil_wrap,
> dummy_true
>  EXT(EXT_subtexture  ,
> dummy_true , GLL,  x ,  x ,  x , 1995)
>  EXT(EXT_tessellation_point_size ,
> ARB_tessellation_shader,  x ,  x ,  x ,  31, 2013)
>  EXT(EXT_tessellation_shader ,
> ARB_tessellation_shader,  x ,  x ,  x ,  31, 2013)
>  EXT(EXT_texture ,
> dummy_true , GLL,  x ,  x ,  x , 1996)
>  EXT(EXT_texture3D   ,
> dummy_true , GLL,  x ,  x ,  x , 1996)
>  EXT(EXT_texture_array   ,
> EXT_texture_array  , GLL, GLC,  x ,  x , 2006)
>  EXT(EXT_texture_border_clamp,
> ARB_texture_border_clamp   ,  x ,  x ,  x , ES2, 2014)
>  EXT(EXT_texture_buffer  ,
> OES_texture_buffer ,  x ,  x ,  x ,  31, 2014)
>  EXT(EXT_texture_compression_dxt1,
> ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2, 2004)
>  EXT(EXT_texture_compression_latc,
> EXT_texture_compression_latc   , GLL,  x ,  x ,  x , 2006)
> -EXT(EXT_texture_compression_rgtc,
> ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x , 2004)
> +EXT(EXT_texture_compression_rgtc,
> ARB_texture_compression_rgtc   , GLL, GLC,  x ,  30, 2004)
>  EXT(EXT_texture_compression_s3tc,
> EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2, 2000)
>  EXT(EXT_texture_cube_map,
> ARB_texture_cube_map   , GLL,  x ,  x ,  x , 2001)
>  EXT(EXT_texture_cube_map_array  ,
> OES_texture_cube_map_array ,  x ,  x ,  x ,  31, 2014)
>  EXT(EXT_texture_edge_clamp  ,
> dummy_true , GLL,  x ,  x ,  x , 1997)
>  EXT(EXT_texture_env_add ,
> dummy_true , GLL,  x ,  x ,  x , 1999)
>  EXT(EXT_texture_env_combine ,
> dummy_true , GLL,  x ,  x ,  x , 2000)
>  EXT(EXT_texture_env_dot3,
> EXT_texture_env_dot3   , GLL,  x ,  x ,  x , 2000)
>  EXT(EXT_texture_filter_anisotropic  ,
> EXT_texture_filter_anisotropic , GLL, GLC, ES1, ES2, 1999)
>  EXT(EXT_texture_format_BGRA ,
> dummy_true ,  x ,  x , ES1, ES2, 2005)
>  EXT(EXT_texture_integer ,
> EXT_texture_integer, GLL, GLC,  x ,  x , 2006)
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 6cdc7818756..bacaa9519ca 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1379,22 +1379,21 @@ _mesa_is_compressed_format(const struct
> gl_context *ctx, GLenum format)
>   return ctx->Extensions.ANGLE_texture_compression_dxt;
>} else {
>   return _mesa_is_desktop_gl(ctx)
>  && ctx->Extensions.EXT_texture_sRGB
>  && ctx->Extensions.EXT_texture_compression_s3tc;
>}
> case MESA_FORMAT_LAYOUT_FXT1:
>return _mesa_is_desktop_gl(ctx)
>   && ctx->Extensions.TDFX_texture_compression_FXT1;
> case MESA_FORMAT_LAYOUT_RGTC:
> -  return _mesa_is_desktop_gl(ctx)
> - && ctx->Extensions.ARB_texture_compression_rgtc;
> +  return ctx->Extensions.ARB_texture_compression_rgtc;

This enables the extension on GLES 1.x as well (same goes for most
hunks in this patch). In fact, wanting to expose this very extension is
what motivated 

Re: [Mesa-dev] Lets talk about autotools

[Timo Aaltonen]

On 27.11.2018 19.42, Eero Tamminen wrote:
> Hi,
> 
> On 27.11.2018 19.05, Matt Turner wrote:
>> On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen 
>> wrote:
>>> On 17.11.2018 6.04, Dylan Baker wrote:
 Quoting Dylan Baker (2018-09-17 09:44:07)
> I feel like for !windows meson is in good enough shape at this
> point that we
> can start having the discussion about deleting the autotools build.
> So, is there
> anything left that autotools can do that meson cannot (that we
> actually want to
> implement)? And, what is a reasonable time-table to remove the
> autotools build?
> I think we could reasonably remove it as soon as 18.3 if others
> felt confident
> that it would work for them.

 Okay, time for an update on things and a chance to talk about what
 else we need.

 Support for llvm-config (and any binary, actually) overriding has
 landed in
 meson, and will be present in the 0.49.0 release, which is due out
 December 9th.
>>>
>>> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
>>> get Mesa backports up until and including 20.0.x, so I wonder how
>>> complex these required new features in meson are to be backported, or
>>> perhaps easily worked around? Backporting a whole new version of meson
>>> might not happen..
>>
>> I understand the LTS concept, but what's the value in never upgrading
>> something like a build tool like Meson? Yeah, new versions give a
>> possibility of regressions, but with something evolving as quickly as
>> Meson the version available in April 2018 becomes less useful for its
>> intended purpose with each passing month...
> 
> Ubuntu has so called hardware enabling (HWE) packages, which provide
> newer versions of kernel, mesa and few other components for LTS, and
> a meta package(s) that pull those in. They're based on package versions
> tested in development versions of Ubuntu.
> 
> For example, relevant Ubuntu 18.10 packages would be available as
> HWE packages for 18.04 somewhere around February, according to
> preliminary Ubuntu kernel schedule.
> 
> Of the packages relevant to Mesa, 18.10 has for example:
> - kernel v4.18   (18.04 has v4.15)
> - LLVM v7.0  (18.04 has LLVM v6)
> - Meson v0.47.2  (18.04 has v0.45)
> 
> I don't know how much that 4 month gap (before new development
> distro release package versions become available in preceding LTS
> release as HW packages) fluctuates.

The backports are being prepared now, bionic-proposed has everything
needed by mesa 18.2.2 (last of the 18.2.x series will be provided
later), xserver and drivers will follow soon. 18.04.2 image will be out
in February '19, but having these in -proposed means the daily image
will use them and gives plenty of time for testing (over the holidays, heh).

Meson is not on the list of HWE backports, but as I said, it could be
backported as a separate source package for mesa to build-depend on if
necessary.


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

Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

[Erik Faye-Lund]

On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
> Jordan Justen  writes:
> 
> > This adds the "Developer's Certificate of Origin 1.1" from the
> > Linux
> > kernel. It indicates that by using Signed-off-by you are certifying
> > that your patch meets the DCO 1.1 guidelines.
> > 
> > It also changes Signed-off-by from being optional to being
> > required.
> > 
> > Signed-off-by: Jordan Justen 
> > Cc: Matt Turner 
> 
> What problem for our project is solved by signed-off-by?  Do you
> think
> that it has actually reduced the incidence of people submitting code
> they don't have permission to submit in the projects where it's been
> used?  I know the behavior in the kernel is that people submit a
> patch,
> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
> just give up.  I don't think anyone stops and says "Wow, that's a
> good
> question.  Maybe I don't have permission to distribute this after
> all?"
> 
> /me sees s-o-b as basically just a linux kernel hazing ritual

I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
reason for the s-o-b is to have a paper-trail for how a patch landed in
the kernel; who it went through etc.

IIRC this came from a lawsuit where the legality of some code came up
for question because it was hard to prove how it made its way into the
kernel.

So the idea is that everyone contributing to putting some code in the
kernel adds a s-o-b, and at least we can ask those people about things
afterwards (and the "legality" is supposed to chain; you can s-o-b a
change because the person before you s-o-b'ed, and you trust the way
you got that s-o-b).

I could imagine Mesa potentially being vulnerable to cases like this,
especially because it contains code based on reverse-engineering; some
GPU vendor might change their view on OSS, and try to claim things are
based on stolen code, for instance. I'm not saying it's likely, just
that it can. And it would really suck if it happened ;)

We also currently have multiple ways for code to get into mesa, but far
fewer ones that the kernel. So I somehow doubt we'd be having an as
hard time with this as the kernel does.

So all in all; I'm not sure it's worth it. But it might be, and it's
not a huge burden IMO.

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

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Erik Faye-Lund]

On Tue, 2018-11-27 at 17:13 -0800, Jordan Justen wrote:
> This documents a mechanism for using GitLab Merge Requests as an
> optional, secondary way to get code reviews for patchsets.
> 
> We still require all patches to be emailed.
> 
> Aside from the potential usage for code review comments, it might
> also
> help reviewers to find unmerged patchsets which need review.
> (Assuming
> it doesn't fall into the same fate of patchwork with hundreds of open
> MRs.)
> 
> Signed-off-by: Jordan Justen 
> Cc: Jason Ekstrand 

In lack of a better thread to discuss this, I'm going to hijack this
one slightly. I hope I'm not too disturbing.

For the better part of the last 4 years, I've been working on a large-
ish (commercial, but open source) project that used GitHub PRs for
contributions. It's a great way of working, for all the reasons we've
already been through:
- Structured reviews
- Easy to get an overview what's happening
- Prevent most bad merges due to CI

But there's a few down-sides as well, compared to mailing list
discussions, and these are the reasons I wanted to speak up. I'm a
strong supporter for MRs, by the way. I just want these issues talked
about. Also note that my experience here comes from GitHub, and not
specifically from GitLab, but I suspect at least some of them apply
here as well:

1. There's usually no reasonable way of reviewing commit messages. You
can add general comments on the development history, but it gets
awkward to navigate between tabs to copy something to quote and stuff.
In e-mail based reviews, you just click reply.

2. It's often difficult to see comments on intermediate commits. At
least on GitHub, intermediate changes gets collapsed as "out-of-date",
because GitHub doesn't seem to care about clean development history.
This tends to 

3. The MR list becomes something that needs to be maintained. Sometimes
there's long-lived MRs that takes months to get merged, or just simply
stall... and they all start piling up. That's fine in a mailing list,
but it quickly gets cumbersome in a MR-based work-flow, as old MRs take
up space in UIs, and makes it harder to see what's going on. So at some
point, stale MRs should be closed IMO. This is going to need someone to
either do it, or to maintain some bot to do it. And some developers
tends to get angry when you close a MR after three months of inactivity
just because nobody wanted to review it.

Out of these, #1 and #2 are my biggest nusances with it, really. The
PR/MR workflow tends to be all about the end-result, and not so much
about the intermediate steps or the "story" they tell. And the UIs
seems to reward such development rather than "the story matters"-
development.

I'm a bit worried that we'd start gradually changing how we developed;
I saw this happen to some degree in my past experience. But at the same
time, I do think that the benefits greatly outweigh the disadvantages.

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

[Mesa-dev] [Bug 107832] Gallium picking A16L16 formats when emulating INTENSITY16 conflicts with mesa

[bugzilla-daemon]

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

Tapani Pälli  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

-- 
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] [PATCH 1/3] intel/blorp: Expand blorp_address::offset to be 64 bits.

[Kenneth Graunke]

In the softpin world, surface state base address may be a fixed 64-bit
address (with no associated BO).  It makes sense to store this in the
offset field.  But it needs to be the full size.

We also update the clear color address to be consistently uint64_t
everywhere so we can continue passing intel_miptree_get_clear_color
a pointer to the blorp_address's offset field without type mismatches.
---
 src/intel/blorp/blorp.h  | 2 +-
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 2 +-
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c| 2 +-
 src/mesa/drivers/dri/i965/intel_mipmap_tree.h| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
index ee343a4a6bb..1e22712602d 100644
--- a/src/intel/blorp/blorp.h
+++ b/src/intel/blorp/blorp.h
@@ -91,8 +91,8 @@ void blorp_batch_finish(struct blorp_batch *batch);
 
 struct blorp_address {
void *buffer;
+   uint64_t offset;
unsigned reloc_flags;
-   uint32_t offset;
uint32_t mocs;
 };
 
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 4daa0e2add1..b067a174056 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -156,7 +156,7 @@ brw_emit_surface_state(struct brw_context *brw,
struct isl_surf *aux_surf = NULL;
uint64_t aux_offset = 0;
struct brw_bo *clear_bo = NULL;
-   uint32_t clear_offset = 0;
+   uint64_t clear_offset = 0;
 
if (aux_usage != ISL_AUX_USAGE_NONE) {
   aux_surf = >aux_buf->surf;
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 8e50aabb3b4..a679ddf3e48 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -3865,7 +3865,7 @@ intel_miptree_get_clear_color(const struct 
gen_device_info *devinfo,
   const struct intel_mipmap_tree *mt,
   enum isl_format view_format, bool sampling,
   struct brw_bo **clear_color_bo,
-  uint32_t *clear_color_offset)
+  uint64_t *clear_color_offset)
 {
assert(mt->aux_buf);
 
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index b0333655ad5..21beeded92a 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -714,7 +714,7 @@ intel_miptree_get_clear_color(const struct gen_device_info 
*devinfo,
   const struct intel_mipmap_tree *mt,
   enum isl_format view_format, bool sampling,
   struct brw_bo **clear_color_bo,
-  uint32_t *clear_color_offset);
+  uint64_t *clear_color_offset);
 
 
 static inline int
-- 
2.19.1

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

[Mesa-dev] [PATCH 2/3] intel/blorp: Make KSP a blorp_address instead of an offset.

[Kenneth Graunke]

In i965, shader programs live in a single buffer, and every batch emits
a STATE_BASE_ADDRESS packet pointing to that buffer.  This takes care of
pinning the buffer for the batch; from then on, we can use an offset.

In the upcoming Iris driver, shader programs can live in multiple
buffers, and those buffers need to be pinned when shader assembly is
referenced.  To aid in this, we turn KSP into a blorp_address rather
than a 32-bit offset.  This lets us also pass a buffer to pin.

For now, we simply assert the BO is NULL and return the offset, as
we did before - nothing should behave differently.
---
 src/intel/blorp/blorp.h |  7 --
 src/intel/blorp/blorp_genX_exec.h   | 27 +
 src/intel/blorp/blorp_priv.h|  6 ++---
 src/intel/vulkan/anv_blorp.c| 10 +---
 src/mesa/drivers/dri/i965/brw_blorp.c   | 18 ++
 src/mesa/drivers/dri/i965/gen4_blorp_exec.h | 12 -
 6 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/src/intel/blorp/blorp.h b/src/intel/blorp/blorp.h
index 1e22712602d..da0c9ac205c 100644
--- a/src/intel/blorp/blorp.h
+++ b/src/intel/blorp/blorp.h
@@ -35,6 +35,7 @@ struct brw_stage_prog_data;
 extern "C" {
 #endif
 
+struct blorp_address;
 struct blorp_batch;
 struct blorp_params;
 
@@ -47,13 +48,15 @@ struct blorp_context {
 
bool (*lookup_shader)(struct blorp_context *blorp,
  const void *key, uint32_t key_size,
- uint32_t *kernel_out, void *prog_data_out);
+ struct blorp_address *kernel_out,
+ void *prog_data_out);
bool (*upload_shader)(struct blorp_context *blorp,
  const void *key, uint32_t key_size,
  const void *kernel, uint32_t kernel_size,
  const struct brw_stage_prog_data *prog_data,
  uint32_t prog_data_size,
- uint32_t *kernel_out, void *prog_data_out);
+ struct blorp_address *kernel_out,
+ void *prog_data_out);
void (*exec)(struct blorp_batch *batch, const struct blorp_params *params);
 };
 
diff --git a/src/intel/blorp/blorp_genX_exec.h 
b/src/intel/blorp/blorp_genX_exec.h
index 065980616ec..20f30c7116d 100644
--- a/src/intel/blorp/blorp_genX_exec.h
+++ b/src/intel/blorp/blorp_genX_exec.h
@@ -108,6 +108,13 @@ _blorp_combine_address(struct blorp_batch *batch, void 
*location,
}
 }
 
+static uint64_t
+KSP(struct blorp_batch *batch, struct blorp_address address)
+{
+   assert(address.buffer == NULL);
+   return address.offset;
+}
+
 #define __gen_address_type struct blorp_address
 #define __gen_user_data struct blorp_batch
 #define __gen_combine_address _blorp_combine_address
@@ -615,7 +622,7 @@ blorp_emit_vs_config(struct blorp_batch *batch,
   if (vs_prog_data) {
  vs.Enable = true;
 
- vs.KernelStartPointer = params->vs_prog_kernel;
+ vs.KernelStartPointer = KSP(batch, params->vs_prog_kernel);
 
  vs.DispatchGRFStartRegisterForURBData =
 vs_prog_data->base.base.dispatch_grf_start_reg;
@@ -795,11 +802,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
  ps.DispatchGRFStartRegisterForConstantSetupData2 =
 brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
 
- ps.KernelStartPointer0 = params->wm_prog_kernel +
+ ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
0);
- ps.KernelStartPointer1 = params->wm_prog_kernel +
+ ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
1);
- ps.KernelStartPointer2 = params->wm_prog_kernel +
+ ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
2);
   }
 
@@ -905,11 +912,11 @@ blorp_emit_ps_config(struct blorp_batch *batch,
  ps.DispatchGRFStartRegisterForConstantSetupData2 =
 brw_wm_prog_data_dispatch_grf_start_reg(prog_data, ps, 2);
 
- ps.KernelStartPointer0 = params->wm_prog_kernel +
+ ps.KernelStartPointer0 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
0);
- ps.KernelStartPointer1 = params->wm_prog_kernel +
+ ps.KernelStartPointer1 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
1);
- ps.KernelStartPointer2 = params->wm_prog_kernel +
+ ps.KernelStartPointer2 = KSP(batch, params->wm_prog_kernel) +
   brw_wm_prog_data_prog_offset(prog_data, ps, 
2);
 
  ps.AttributeEnable = prog_data->num_varying_inputs > 0;
@@ -973,11 

[Mesa-dev] [PATCH 3/3] intel/blorp: Add a simpler interface for softpin-only drivers.

[Kenneth Graunke]

Drivers using softpin do not need (or have) relocations.  The old
relocation interface is somewhat awkward and limiting.

In particular, brw_surface_reloc assumes that all SURFACE_STATEs will
exist in a single buffer, and only provides ss_offset.  The driver is
supposed to implicitly know about this buffer, in order to offset from
a CPU map of that buffer to write the value.  With softpin, we can put
SURFACE_STATEs in any buffer we like, as long as it's within 4GB of
Surface State Base Address.  So, this model breaks down.

Drivers can now #define BLORP_USE_SOFTPIN and define a simpler
blorp_use_pinned_bo() helper, which adds the buffer to the validation
list for the current batch, and returns the (statically assigned,
unchanging) address for the buffer.

The upcoming Iris driver will use this interface.
---
 src/intel/blorp/blorp_genX_exec.h | 36 +++
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/intel/blorp/blorp_genX_exec.h 
b/src/intel/blorp/blorp_genX_exec.h
index 20f30c7116d..f18de3dc6ce 100644
--- a/src/intel/blorp/blorp_genX_exec.h
+++ b/src/intel/blorp/blorp_genX_exec.h
@@ -47,10 +47,27 @@
 static void *
 blorp_emit_dwords(struct blorp_batch *batch, unsigned n);
 
+#ifdef BLORP_USE_SOFTPIN
+static uint64_t
+blorp_use_pinned_bo(struct blorp_batch *batch, struct blorp_address addr);
+
+/* Wrappers to avoid #ifdefs everywhere */
+#define blorp_emit_reloc _blorp_combine_address
+static inline void
+blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset,
+struct blorp_address address, uint32_t delta)
+{
+}
+#else
 static uint64_t
 blorp_emit_reloc(struct blorp_batch *batch,
  void *location, struct blorp_address address, uint32_t delta);
 
+static void
+blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset,
+struct blorp_address address, uint32_t delta);
+#endif
+
 static void *
 blorp_alloc_dynamic_state(struct blorp_batch *batch,
   uint32_t size,
@@ -78,10 +95,6 @@ blorp_alloc_binding_table(struct blorp_batch *batch, 
unsigned num_entries,
 static void
 blorp_flush_range(struct blorp_batch *batch, void *start, size_t size);
 
-static void
-blorp_surface_reloc(struct blorp_batch *batch, uint32_t ss_offset,
-struct blorp_address address, uint32_t delta);
-
 #if GEN_GEN >= 7 && GEN_GEN < 10
 static struct blorp_address
 blorp_get_surface_base_address(struct blorp_batch *batch);
@@ -104,15 +117,23 @@ _blorp_combine_address(struct blorp_batch *batch, void 
*location,
if (address.buffer == NULL) {
   return address.offset + delta;
} else {
+#ifdef BLORP_USE_SOFTPIN
+  return blorp_use_pinned_bo(batch, address) + delta;
+#else
   return blorp_emit_reloc(batch, location, address, delta);
+#endif
}
 }
 
 static uint64_t
 KSP(struct blorp_batch *batch, struct blorp_address address)
 {
+#ifdef BLORP_USE_SOFTPIN
+   return blorp_use_pinned_bo(batch, address);
+#else
assert(address.buffer == NULL);
return address.offset;
+#endif
 }
 
 #define __gen_address_type struct blorp_address
@@ -1370,6 +1391,13 @@ blorp_emit_surface_state(struct blorp_batch *batch,
isl_surf_fill_state(batch->blorp->isl_dev, state,
.surf = , .view = >view,
.aux_surf = >aux_surf, .aux_usage = aux_usage,
+#ifdef BLORP_USE_SOFTPIN
+   .address = blorp_use_pinned_bo(batch, surface->addr),
+   .aux_address = aux_usage != ISL_AUX_USAGE_NONE ?
+  blorp_use_pinned_bo(batch, surface->aux_addr) : 0,
+   .clear_address = !use_clear_address ? 0 :
+  blorp_use_pinned_bo(batch, 
surface->clear_color_addr),
+#endif
.mocs = surface->addr.mocs,
.clear_color = surface->clear_color,
.use_clear_address = use_clear_address,
-- 
2.19.1

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