Re: Mesa >= 23.3.x and python 2.6 ...

2024-01-19 Thread Matt Turner
On Thu, Jan 18, 2024 at 10:22 AM Stefan Dirsch  wrote:
> I noticed that with version 23.3.x Mesa no longer can be built with python
> 2.6. It still worked with Mesa 23.2.1.

For anyone who got this far and was completely incredulous... this
(and the subject) is typo'd -- the problem is about Python 3.6, not
2.6.


Re: Spam

2023-08-30 Thread Matt Turner
On Mon, Aug 28, 2023 at 8:25 AM Mike Blumenkrantz
 wrote:
> Hi,
>
> As everyone has likely noticed, we've had a significant uptick of spam on 
> Mesa-related IRC channels lately. Sometimes these occurrences go on for many 
> hours before someone takes action.
>
> I'd like to request that all commonly-affected channels (#dri-devel, 
> #intel-3d, #freedesktop, ???) take some time over the next week and staff up 
> on more moderators for greater coverage.

It seems absurd that we cannot prevent a single Estonian man (with
mental health problems?) from spamming our channels for more than 10
years across two IRC networks.

Here's a similar thread from llvm-dev about the same person:
https://groups.google.com/g/llvm-dev/c/VaFMoTRdt40/m/k4EmXJRqBgAJ

Do we have any information about the real-life person? If so, maybe we
can contact their ISP or something?


Gmail/GitLab email filtering script

2021-12-29 Thread Matt Turner
In case it's useful for anyone, I've written a Google Apps Script that
allows me to filter notification emails based on X-GitLab headers.
It's available here
(https://github.com/mattst88/gmail-gitlab-filtering) and I'd be happy
to hear if it's useful for you.

Thanks,
Matt


Re: [Mesa-dev] Let's enable _GLIBCXX_ASSERTIONS=1 on mesa debug builds

2021-09-10 Thread Matt Turner
On Fri, Sep 10, 2021 at 2:19 PM Timur Kristóf  wrote:
> Matt Turner  ezt írta (időpont: 2021. szept. 10., P 
> 22:33):
>>
>> On Fri, Sep 10, 2021 at 10:20 AM Timur Kristóf  
>> wrote:
>> >
>> > Hi,
>> >
>> > We've been recently working on tracking down some "mysterious" crashes
>> > that some users experienced on distro builds of mesa but we couldn't
>> > reproduce locally, until we found out about _GLIBCXX_ASSERTIONS=1 which
>> > seems to be not enabled by default in mesa, but is enabled by a lot of
>> > distros.
>> >
>> > I realize that enabling it by default on all mesa builds would have
>> > performance implications, so I propose to just enable it by default in
>> > mesa debug builds.
>> >
>> > What do you think? Would this be okay with the mesa community?
>>
>> I've never heard of this before. According to the documentation [1] it is:
>>
>> > _GLIBCXX_ASSERTIONS
>> >
>> > Undefined by default. When defined, enables extra error checking in the 
>> > form of precondition assertions, such as bounds checking in strings and 
>> > null pointer checks when dereferencing smart pointers.
>>
>> Seems reasonable to enable it for debug builds if we're using C++
>> features in Mesa that it covers.
>
>
> While at it, do you think we should also use any of the other similar macros 
> from the doc?
>
> I think _GLIBCXX_DEBUG and _GLIBCXX_SANITIZE_VECTOR look particularly useful.

Yeah, those seem reasonable as well. Don't know if we have the ability
to enable _GLIBCXX_SANITIZE_VECTOR only with ASan or not (or whether
it does something without ASan also enabled), but that might be worth
looking into.


Re: [Mesa-dev] Let's enable _GLIBCXX_ASSERTIONS=1 on mesa debug builds

2021-09-10 Thread Matt Turner
On Fri, Sep 10, 2021 at 10:20 AM Timur Kristóf  wrote:
>
> Hi,
>
> We've been recently working on tracking down some "mysterious" crashes
> that some users experienced on distro builds of mesa but we couldn't
> reproduce locally, until we found out about _GLIBCXX_ASSERTIONS=1 which
> seems to be not enabled by default in mesa, but is enabled by a lot of
> distros.
>
> I realize that enabling it by default on all mesa builds would have
> performance implications, so I propose to just enable it by default in
> mesa debug builds.
>
> What do you think? Would this be okay with the mesa community?

I've never heard of this before. According to the documentation [1] it is:

> _GLIBCXX_ASSERTIONS
>
> Undefined by default. When defined, enables extra error checking in the form 
> of precondition assertions, such as bounds checking in strings and null 
> pointer checks when dereferencing smart pointers.

Seems reasonable to enable it for debug builds if we're using C++
features in Mesa that it covers.

[1] https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html


[Mesa-dev] [ANNOUNCE] glu 9.0.2

2021-06-24 Thread Matt Turner
Big thanks to Dylan for the Meson build system!

Dylan Baker (3):
  editorconfig: Add configuration for Meson
  Add Meson build system
  Makefile: Distribute Meson files as part of the dist tarball

Kevin Bowling (1):
  build: Add support for libglvnd

Lucas Stach (1):
  build: fix the debug parameter to properly handle --disable-debug

Matt Turner (2):
  Remove glu_mangle.h
  glu 9.0.2

Nicolas Caramelli (1):
  Check the definition instead of the extension to which it belongs

git tag: glu-9.0.2

https://mesa.freedesktop.org/archive/glu/glu-9.0.2.tar.gz
SHA256: 24effdfb952453cc00e275e1c82ca9787506aba0282145fff054498e60e19a65  
glu-9.0.2.tar.gz
SHA512: 
5653ae7be7c580cbcd589061667e35b611bc7c43fccf7dd3f6c9c286a19052c1fbc8ade3ef435bda6e9ef6304248a12cc67c0603015692b5158ed4d6327be7d5
  glu-9.0.2.tar.gz
PGP:  https://mesa.freedesktop.org/archive/glu/glu-9.0.2.tar.gz.sig

https://mesa.freedesktop.org/archive/glu/glu-9.0.2.tar.xz
SHA256: 6e7280ff585c6a1d9dfcdf2fca489251634b3377bfc33c29e4002466a38d02d4  
glu-9.0.2.tar.xz
SHA512: 
2517d7406bb643d12c017a95dcb5d8716f307344332638bcbdf274a90752a7c22165d34745f1b082ed916bb07d40e62d1d1d67d96426225be63166f3480d6f64
  glu-9.0.2.tar.xz
PGP:  https://mesa.freedesktop.org/archive/glu/glu-9.0.2.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


Re: [Mesa-dev] Rust drivers in Mesa

2020-10-05 Thread Matt Turner
On Sun, Oct 4, 2020 at 2:00 PM Marek Olšák  wrote:
> I think it's just going to get more messy and complicated for people who 
> don't want to learn or use another language. Mesa already requires people to 
> know C, Python, and now newly Gitlab CI scripts just to get stuff done and 
> merged. Another language would only exacerbate the issue and steepen the 
> learning curve.

To some extent I agree, and I think that's a good reason to start with
Rust in a leaf-node of the project (a driver or compiler backend)
rather than in a common piece of infrastructure.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] gitlab.fd.o financial situation and impact on services

2020-02-28 Thread Matt Turner
On Fri, Feb 28, 2020 at 12:00 AM Daniel Stone  wrote:
>
> Hi Matt,
>
> On Thu, 27 Feb 2020 at 23:45, Matt Turner  wrote:
> > We're paying 75K USD for the bandwidth to transfer data from the
> > GitLab cloud instance. i.e., for viewing the https site, for
> > cloning/updating git repos, and for downloading CI artifacts/images to
> > the testing machines (AFAIU).
>
> I believe that in January, we had $2082 of network cost (almost
> entirely egress; ingress is basically free) and $1750 of cloud-storage
> cost (almost all of which was download). That's based on 16TB of
> cloud-storage (CI artifacts, container images, file uploads, Git LFS)
> egress and 17.9TB of other egress (the web service itself, repo
> activity). Projecting that out gives us roughly $45k of network
> activity alone, so it looks like this figure is based on a projected
> increase of ~50%.
>
> The actual compute capacity is closer to $1150/month.

Could we have the full GCP bill posted?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] gitlab.fd.o financial situation and impact on services

2020-02-27 Thread Matt Turner
On Thu, Feb 27, 2020 at 1:27 PM Daniel Vetter  wrote:
>
> Hi all,
>
> You might have read the short take in the X.org board meeting minutes
> already, here's the long version.
>
> The good news: gitlab.fd.o has become very popular with our
> communities, and is used extensively. This especially includes all the
> CI integration. Modern development process and tooling, yay!
>
> The bad news: The cost in growth has also been tremendous, and it's
> breaking our bank account. With reasonable estimates for continued
> growth we're expecting hosting expenses totalling 75k USD this year,
> and 90k USD next year. With the current sponsors we've set up we can't
> sustain that. We estimate that hosting expenses for gitlab.fd.o
> without any of the CI features enabled would total 30k USD, which is
> within X.org's ability to support through various sponsorships, mostly
> through XDC.
>
> Note that X.org does no longer sponsor any CI runners themselves,
> we've stopped that. The huge additional expenses are all just in
> storing and serving build artifacts and images to outside CI runners
> sponsored by various companies. A related topic is that with the
> growth in fd.o it's becoming infeasible to maintain it all on
> volunteer admin time. X.org is therefore also looking for admin
> sponsorship, at least medium term.
>
> Assuming that we want cash flow reserves for one year of gitlab.fd.o
> (without CI support) and a trimmed XDC and assuming no sponsor payment
> meanwhile, we'd have to cut CI services somewhere between May and June
> this year. The board is of course working on acquiring sponsors, but
> filling a shortfall of this magnitude is neither easy nor quick work,
> and we therefore decided to give an early warning as soon as possible.
> Any help in finding sponsors for fd.o is very much appreciated.

Some clarification I got from Daniel in a private conversation, since
I was confused about what the money was paying for exactly:

We're paying 75K USD for the bandwidth to transfer data from the
GitLab cloud instance. i.e., for viewing the https site, for
cloning/updating git repos, and for downloading CI artifacts/images to
the testing machines (AFAIU).

I was not aware that we were being charged for anything wrt GitLab
hosting yet (and neither was anyone on my team at Intel that I've
asked). This... kind of needs to be communicated.

A consistent concern put forth when we were discussing switching to
GitLab and building CI was... how do we pay for it. It felt like that
concern was always handwaved away. I heard many times that if we
needed more runners that we could just ask Google to spin up a few
more. If we needed testing machines they'd be donated. No one
mentioned that all the while we were paying for bandwidth... Perhaps
people building the CI would make different decisions about its
structure if they knew it was going to wipe out the bank account.

What percentage of the bandwidth is consumed by transferring CI
images, etc? Wouldn't 75K USD would be enough to buy all the testing
machines we need and host them within Google or wherever so we don't
need to pay for huge amounts of bandwidth?

I understand that self-hosting was attractive so that we didn't find
ourselves on the SourceForge-equivalent hosting platform of 2022, but
is that risk real enough to justify spending 75K+ per year? If we were
hosted on gitlab.com or github.com, we wouldn't be paying for
transferring CI images to CI test machines, etc, would we?

So what do we do now? Have we painted ourselves into a corner?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Is it time to stop using the mailing list for patch review?

2019-12-10 Thread Matt Turner
On Mon, Dec 9, 2019 at 6:07 PM Dylan Baker  wrote:
>
> Hi everyone,
>
> I think its time we discussed whether we're going to continue to do patch 
> review
> on the mailing list, or if it it should all go through gitlab. I think we 
> should
> stop using the mailing list, here are some reasons:
>
> 1) Most development is happening on gitlab at this point, patches on the 
> mailing
>list are often overlooked
> 2) The mailing list bypasses CI which potentially breaks the build
> 3) Probably more reasons I'm forgetting.

I think effectively we're already there.

What concrete change would you propose?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Moving libglvnd to FreeDesktop Gitlab

2019-09-24 Thread Matt Turner
On Wed, Sep 11, 2019 at 9:14 PM Kyle Brenneman  wrote:
>
> On 9/9/19 12:07 PM, Adam Jackson wrote:
> > On Wed, 2019-09-04 at 14:27 -0600, Kyle Brenneman wrote:
> >> On 9/4/19 8:44 AM, Daniel Stone wrote:
> >>> Hi,
> >>>
> >>> On Wed, 4 Sep 2019 at 15:12, Chuck Atkins  
> >>> wrote:
>  Can we use Gitlab's GitHub import feature?
> 
>  https://gitlab.freedesktop.org/help/user/project/import/github.md
> 
>  I haven't used it before but it looks like it will migrate everything, 
>  i.e. repo, issues, prs, etc.
> >>> Yeah, we definitely can. We can create a new namespace for GLVND and
> >>> import the project into there.
> >>>
> >>> Who else should I add to the group?
> >> I'm not very familiar with the administrative side of GitLab -- does
> >> adding to the group just control who can check in new commits?
> > It's fairly fine-grained, for details see:
> >
> > https://docs.gitlab.com/ee/user/permissions.html
> >
> > - ajax
> >
> In that case, in addition to myself and the other NVIDIA engineers, Adam
> Jackson would probably make sense to add for push/merge permission. Not
> sure who else.
>
> Is there anything that's needed from me to get the GitHub repo ready to
> import?
>
> -Kyle

Okay, everyone is in agreement. Great!

Mesa 19.2.0 is going to be released tomorrow and has been prepared for
libglvnd finally having pkgconfig files after many years of waiting,
but there's no libglvnd release for distros to ship. Is there
something we're waiting on?

Can we make a release, transition to FDO Gitlab, etc? Daniel says that
only someone with permissions on the GitHub repo (Kyle, I suppose) can
do the transition.

Can we please make this happen tomorrow?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-11 Thread Matt Turner
On Tue, Sep 10, 2019 at 10:54 PM Brian Paul  wrote:
>
> IIRC, designated initializers are not legal C++.
> Fixes the MSVC build.
>
> Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
> ---
>  src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 280a778..d6a0264 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
>  */
> if (shader_program->data->spirv) {
>static const gl_nir_linker_options opts = {
> - .fill_parameters = true,
> + true /*fill_parameters */

Probably intended to have a space after /*
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] 19.2 release train going off the rails

2019-09-03 Thread Matt Turner
What is going on with the 19.2 RCs? I know that we said we would push
the releases back a week to let some feature work land, but the last
blocker of the feature tracker [0] landed on the 14th and RC1 wasn't
until the 20th. Now another two weeks have passed without an RC. RC2
should have been tagged last week and RC3 this week.

Can someone take over 19.2 and get this rolling again?

Thanks,
Matt

[0] https://bugs.freedesktop.org/show_bug.cgi?id=111265
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Moving libglvnd to FreeDesktop Gitlab

2019-08-31 Thread Matt Turner
Getting patches into libglvnd has proven quite difficult (see [0] for
example). There was some talk of moving it to FreeDesktop Gitlab on
IRC recently. Can we move forward with that? Are there objections to
doing so?

[0] https://github.com/NVIDIA/libglvnd/pull/86
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa 19.2.0 release plan

2019-08-18 Thread Matt Turner
On Thu, Aug 8, 2019 at 2:56 AM Emil Velikov  wrote:
>
> On Wed, 7 Aug 2019 at 21:43, Mark Janes  wrote:
> >
> > Eric Engestrom  writes:
> >
> > > On 2019-07-31 at 09:38, Emil Velikov  wrote:
> > >> Hi all,
> > >>
> > >> Here is the tentative release plan for 19.2.0.
> > >>
> > >> As many of you are well aware, it's time to the next branch point.
> > >> The calendar is already updated, so these are the tentative dates:
> > >>
> > >>  Aug 06 2019 - Feature freeze/Release candidate 1
> > >>  Aug 13 2019 - Release candidate 2
> > >>  Aug 20 2019 - Release candidate 3
> > >>  Aug 27 2019 - Release candidate 4/final release
> > >>
> > >> This gives us around 1 week until the branch point.
> > >>
> > >> Note: In the spirit of keeping things clearer and more transparent, we
> > >> will be keeping track of any features planned for the release in
> > >> Bugzilla [1].
> > >>
> > >> Do add a separate "Depends on" for each work you have planned.
> > >> Alternatively you can reply to this email and I'll add them for you.
> > >>
> > >> [1] https://bugs.freedesktop.org/show_bug.cgi?id=111265
> > >
> > > Thanks!
> > >
> > > As per previous discussions (I don't remember where, sorry) as well as
> > > internal discussions, I think we should add all currently open
> > > regressions since 19.1 as blockers for this release.
> >
> > My understanding is that the "feature tracker" blocks the creation of
> > the release branchpoint.  A separate "release tracker" blocks the
> > release of 19.2.0.  Unfixed regressions go on the "release tracker", not
> > the "feature tracker".  We backport bug fixes to release branches, but
> > we don't backport features.
> >
> Yes that is correct. We are interested in features for the next few days.
> Afterwords we'll focus on bugfixes.

The last bug in the feature tracker was closed on the 14th. Can we make RC1 now?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/ir: Fix CFG corruption in opt_predicated_break().

2019-08-01 Thread Matt Turner
On Tue, Jul 23, 2019 at 5:58 PM Francisco Jerez  wrote:
>
> Specifically the optimization of a conditional BREAK + WHILE sequence
> into a conditional WHILE seems pretty broken.  The list of successors
> of "earlier_block" (where the conditional BREAK was found) is emptied
> and then re-created with the same edges for no apparent reason.  On
> top of that the list of predecessors of the block immediately after
> the WHILE loop is emptied, but only one of the original edges will be
> added back, which means that potentially several blocks that still
> have it on their list of successors won't be on its list of
> predecessors anymore, causing all sorts of hilarity due to the
> inconsistency in the control flow graph.

It's been ~5 years since I wrote the code, but I think the idea was to
prevent ::combine_with from ever combining blocks that had "internal"
edges -- that is, the first block cannot have outgoing edges and the
second block cannot have incoming edges. That still seems to me to be
a good idea. I guess it would be fine if that were relaxed to be the
only

The intention was to ensure that in the rare cases that we use
::combine_with that the programmer has to think hard about how they're
rewiring the CFG.

> The solution is to remove the code that's removing valid edges from
> the CFG.  cfg_t::remove_block() will already clean up after itself.
> The assert in bblock_t::combine_with() also needs to be removed since
> we will be merging a block with multiple children into the first one
> of them.

Okay, so I think you're saying that ::remove_block already does
exactly what we need so we don't need to bother ensuring that the
"internal" edges are removed. If that's an accurate restatement of
your claim then this patch makes sense to me and is

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

[Mesa-dev] [ANNOUNCE] glu 9.0.1

2019-07-23 Thread Matt Turner

Amarnath Valluri (1):
 libutils/mipmap.c: Fixed possible memory leak

John Hein (1):
 pkgconfig: Include -I path for glu itself

Krzysztof Kosiński (1):
 Remove all uses of the register keyword.

Matt Turner (4):
 Add -D(N)DEBUG to CFLAGS dependent on --enable-debug
 libutil: Include stddef.h for NULL
 Switch from bz2 to xz
 glu 9.0.1

Mike Gorchak (1):
 glu: initialize PriorityQ::order field to NULL in pqNewPriorityQ()

git tag: glu-9.0.1

https://mesa.freedesktop.org/archive/glu/glu-9.0.1.tar.gz
MD5:  5599a0e0a97335e10239d9165aced60d  glu-9.0.1.tar.gz
SHA1: 2d1388106d2556a3847ae7404d7d9dba6531755b  glu-9.0.1.tar.gz
SHA256: f6f484cfcd51e489afe88031afdea1e173aa652697e4c19ddbcb8260579a10f7  
glu-9.0.1.tar.gz
SHA512: 
31d5ae196a42df61a6b161f6107049dbcb59c1517d18dd106324297543b90cff5f0a0720328364f9befaeb7f36d8425ec37b05dfa33f1b750cbcda423888f71e
  glu-9.0.1.tar.gz
PGP:  https://mesa.freedesktop.org/archive/glu/glu-9.0.1.tar.gz.sig

https://mesa.freedesktop.org/archive/glu/glu-9.0.1.tar.xz
MD5:  151aef599b8259efe9acd599c96ea2a3  glu-9.0.1.tar.xz
SHA1: b6ffef562ba55d3f80146d4238589cb9b1de66f5  glu-9.0.1.tar.xz
SHA256: fb5a4c2dd6ba6d1c21ab7c05129b0769544e1d68e1e3b0ffecb18e73c93055bc  
glu-9.0.1.tar.xz
SHA512: 
8a6dae5b4bd63efb96d15f23ccda4ad9c2ffaa964897e5fa63d1e58360d8d4e6732c5efd2109dba04155d5fc457ab1718a65cf9b544ce0d452679ba988d04018
  glu-9.0.1.tar.xz
PGP:  https://mesa.freedesktop.org/archive/glu/glu-9.0.1.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

Re: [Mesa-dev] [PATCH 2/2] meson: make "auto" only choose buildable drivers

2019-07-19 Thread Matt Turner
On Thu, Jul 18, 2019 at 7:19 AM Alyssa Ross  wrote:
>
> "auto" pays attention to the OS and architecture of the target system,
> but not the available libraries. If, say, libdrm isn't available, "auto"
> won't work, and a manual list of drivers will be required anyway. It
> would also try building the virgl and svga gallium drivers, even when
> unsupported due to building with EGL and no compatible platform.

Today with "auto" you'll get some error message if a driver cannot be
built because of a missing library, right?

What happens after your patch? Is the user informed that some driver
included in the "auto" set is not going to be built because of a
missing library? It doesn't seem like it, unless I'm missing
something, and that seems potentially worse than giving an error.

I think automatic selection of build targets really breaks down when
you have as many options and dependencies as we have.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/fs/ra: Stop adding RA interference to too many SENDS nodes

2019-05-09 Thread Matt Turner
Heh, interesting!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/fs/ra: Stop adding RA interference to too many SENDS nodes

2019-05-08 Thread Matt Turner
Whoops/Nice!

Are there any shader-db changes as a result?

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

[Mesa-dev] [PATCH] intel/compiler: Unset flag reg when FB write is not predicated

2019-04-29 Thread Matt Turner
In the FS IR we pretend that the instruction is predicated with (+f0.1)
just for flag dependency tracking purposes. Since the instruction
doesn't support predication before Haswell, we unset the predicate so we
should also unset the flag register so that we can round-trip the
disassembly.
---
 src/intel/compiler/brw_fs_generator.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/intel/compiler/brw_fs_generator.cpp 
b/src/intel/compiler/brw_fs_generator.cpp
index af8350aed6c..84909f83fec 100644
--- a/src/intel/compiler/brw_fs_generator.cpp
+++ b/src/intel/compiler/brw_fs_generator.cpp
@@ -363,6 +363,7 @@ fs_generator::generate_fb_write(fs_inst *inst, struct 
brw_reg payload)
 {
if (devinfo->gen < 8 && !devinfo->is_haswell) {
   brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
+  brw_set_default_flag_reg(p, 0, 0);
}
 
const struct brw_reg implied_header =
-- 
2.21.0

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

Re: [Mesa-dev] Add an ASSERTED macro to use in place of MAYBE_UNUSED?

2019-04-22 Thread Matt Turner
On Mon, Apr 22, 2019 at 1:09 PM Kristian Høgsberg  wrote:
>
> On Mon, Apr 22, 2019 at 12:11 PM Jason Ekstrand  wrote:
> >
> > All,
> >
> > I've seen discussions come up several times lately about whether you should 
> > use MAYBE_UNUSED or UNUSED in what scenario and why do we have two of them 
> > anyway.  That got me thinking a bit.  Maybe what we actually want instead 
> > of MAYBE_UNUSED is something like this:
> >
> > #ifdef NDEBUG
> > #define ASSERTED UNUSED
> > #else
> > #define ASSERTED
> > #endif
> >
> > That way, if you only need a parameter for asserts, you can declare it 
> > ASSERTED and it won't warn in release builds will still throw a warning if 
> > you do a debug build which doesn't use it.  Of course, there are other 
> > times when something is validly MAYBE_UNUSED such as auto-generated code or 
> > the genX code we use on Intel.  However, this provides additional meaning 
> > and means the compiler warnings are still useful even after you've 
> > relegated a value to assert-only.
> >
> > Thoughts?  I'm also open to a better name; that's the best I could do in 5 
> > minutes.
>
> I think that's going in the wrong direction - if anything I think that
> having both UNUSED and MAYBE_UNUSED is redundant and feel that just
> UNUSED would be fine. __attribute__((unused)) doesn't mean "strictly
> not used", it means "don't warn if this isn't used".

I agree that having both UNUSED and MAYBE_UNUSED is silly and I would
be happy to see MAYBE_UNUSED go away.

I think the advantage of Jason's proposal is that we are alerted if
there is actually dead code. E.g., if we remove the assert that used a
variable, we currently won't get a warning from the compiler that the
variable is unused. At least in release builds we would, if we did
what Jason suggests.

Maybe we do what Jason suggests and then remove MAYBE_UNUSED?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [ANNOUNCE] Mesa 18.3.6 release candidate

2019-04-04 Thread Matt Turner
On Thu, Apr 4, 2019 at 9:16 AM Emil Velikov  wrote:
>
> On Tue, 2 Apr 2019 at 20:13, Matt Turner  wrote:
> >
> > On Mon, Apr 1, 2019 at 5:18 AM Emil Velikov  
> > wrote:
> > > For anyone wondering about the delay:
> > >
> > > We have been using LunarG OpenGL and Vulkan testing service to
> > > validate Mesa releases since day 1.
> > >
> > > Unfortunately, they've been experiencing some issues which we're
> > > expecting to be resolved any day now.
> >
> > We have? Who is we? Dylan doesn't (and his releases make it out on time).
> >
> Feel free to login to share.lunarg.com and observe. As of 2 minutes
> ago, all the results are still not available.

That's not really what I'm concerned about though.

> I've mentioned and recommended it to other release managers, yet seems
> like I've never documented that :-\
>
> AFAICT it's the only piece that runs actual games (traces) as seen in the 
> wild.

I mean, that's great but at no point did the Mesa community decide to
gate releases on the results of this system.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [ANNOUNCE] Mesa 18.3.6 release candidate

2019-04-02 Thread Matt Turner
On Mon, Apr 1, 2019 at 5:18 AM Emil Velikov  wrote:
> For anyone wondering about the delay:
>
> We have been using LunarG OpenGL and Vulkan testing service to
> validate Mesa releases since day 1.
>
> Unfortunately, they've been experiencing some issues which we're
> expecting to be resolved any day now.

We have? Who is we? Dylan doesn't (and his releases make it out on time).

The 18.3.x release train has gone off the rails repeatedly. We should
not be holding releases up because of the LunarG testing service
without prior discussion.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/compiler: Replicate 16 bit immediate value correctly

2019-03-26 Thread Matt Turner
On Tue, Mar 26, 2019 at 3:35 PM Sagar Ghuge  wrote:
>
> For the W or UW (signed or unsigned word) source types, the 16-bit value
> must be replicated in both the low and high words of the 32-bit
> immediate value.
>
> Signed-off-by: Sagar Ghuge 
> ---
>  src/intel/compiler/brw_fs.cpp | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 0c2439d9daf..f8cb91fcf21 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -4069,6 +4069,9 @@ fs_visitor::lower_integer_multiplication()
>  mul->src[1].type = BRW_REGISTER_TYPE_UW;
>  mul->src[1].stride *= 2;
>
> +if (mul->src[1].file == IMM)
> +   mul->src[1].ud = ((mul->src[1].ud & 0x) |
> + mul->src[1].ud << 16);

Please put braces around the statement, since it's in nested control flow.

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

Re: [Mesa-dev] Mesa CI is too slow

2019-02-18 Thread Matt Turner
On Mon, Feb 18, 2019 at 9:32 AM Daniel Stone  wrote:
>
> Hi all,
> A few people have noted that Mesa's GitLab CI is just too slow, and
> not usable in day-to-day development, which is a massive shame.
>
> I looked into it a bit this morning, and also discussed it with Emil,
> though nothing in this is speaking for him.
>
> Taking one of the last runs as representative (nothing in it looks
> like an outlier to me, and 7min to build RadeonSI seems entirely
> reasonable):
> https://gitlab.freedesktop.org/mesa/mesa/pipelines/19692/builds
>
> This run executed 24 jobs, which is beyond the limit of our CI
> parallelism. As documented on
> https://www.freedesktop.org/wiki/Infrastructure/ we have 14 concurrent
> job slots (each with roughly 4 vCPUs). Those 24 jobs cumulatively took
> 177 minutes of execution time, taking 120 minutes for the end-to-end
> pipeline.
>
> 177 minutes of runtime is too long for the runners we have now: if it
> perfectly occupies all our runners it will take over 12 minutes, which
> means that even if no-one else was using the runners, they could
> execute 5 Mesa builds per hour at full occupancy. Unfortunately,
> VirGL, Wayland/Weston, libinput, X.Org, IGT, GStreamer,
> NetworkManager/ModemManager, Bolt, Poppler, etc, would all probably
> have something to say about that.
>
> When the runners aren't occupied and there's less contention for jobs,
> it looks quite good:
> https://gitlab.freedesktop.org/anholt/mesa/pipelines/19621/builds
>
> This run 'only' took 20.5 minutes to execute, but then again, 3
> pipelines per hour isn't that great either.
>
> Two hours of end-to-end pipeline time is also obviously far too long.
> Amongst other things, it practically precludes pre-merge CI: by the
> time your build has finished, someone will have pushed to the tree, so
> you need to start again. Even if we serialised it through a bot, that
> would limit us to pushing 12 changesets per day, which seems too low.
>
> I'm currently talking to two different hosts to try to get more
> sponsored time for CI runners. Those are both on hold this week due to
> travel / personal circumstances, but I'll hopefully find out more next
> week. Eric E filed an issue
> (https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/120) to
> enable ccache cache but I don't see myself having the time to do it
> before next month.
>
> In the meantime, it would be great to see how we could reduce the
> number of jobs Mesa runs for each pipeline. Given we're already
> exceeding the limits of parallelism, having so many independent jobs
> isn't reducing the end-to-end pipeline time, but instead just
> duplicating effort required to fetch and check out sources, cache (in
> the future), start the container, run meson or ./configure, and build
> any common files.
>
> I'm taking it as a given that at least three separate builds are
> required: autotools, Meson, and SCons. Fair enough.
>
> It's been suggested to me that SWR should remain separate, as it takes
> longer to build than the other drivers, and getting fast feedback is
> important, which is fair enough.
>
> Suggestion #1: merge scons-swr into scons-llvm. scons-nollvm will
> already provide fast feedback on if we've broken the SCons build, and
> the rest is pretty uninteresting, so merging scons-swr into scons-llvm
> might help cut down on duplication.
>
> Suggestion #2: merge the misc Gallium jobs together. Building
> gallium-radeonsi and gallium-st-other are both relatively quick. We
> could merge these into gallium-drivers-other for a very small increase
> in overall runtime for that job, and save ourselves probably about 10%
> of the overall build time here.
>
> Suggestion #3: don't build so much LLVM in autotools. The Meson
> clover-llvm builds take half the time the autotools builds do. Perhaps
> we should only build one LLVM variant within autotools (to test the
> autotools LLVM selection still works), and then build all the rest
> only in Meson. That would be good for another 15-20% reduction in
> overall pipeline run time.
>
> Suggestion #4 (if necessary): build SWR less frequently. Can we
> perhaps demote SWR to an 'only:' job which will only rebuild SWR if
> SWR itself or Gallium have changed? This would save a good chunk of
> runtime - again close to 10%.
>
> Doing the above would reduce the run time fairly substantially, for
> what I can tell is no loss in functional coverage, and bring the
> parallelism to a mere 1.5x oversubscription of the whole
> organisation's available job slots, from the current 2x.
>
> Any thoughts?

All of your suggestions seem reasonable.

Removing autotools [1] would obviously reduce the number of builds.

If I understood correctly, we are kicking off a CI run for every push
to a fork of the Mesa repo, and not just for merge requests. I think
that's absolutely the wrong thing to do. CI for personal branches
should be opt-in.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=mesa-autotools-removal

Re: [Mesa-dev] [PATCH 4/5] intel/fs: Implement extended strides greater than 4 for IR source regions.

2019-02-14 Thread Matt Turner
On Thu, Feb 14, 2019 at 1:30 PM Jason Ekstrand  wrote:
>
> On Fri, Jan 18, 2019 at 6:09 PM Francisco Jerez  wrote:
>>
>> Strides up to 32B can be implemented for the source regions of most
>> instructions by leveraging either the vertical or the horizontal
>> stride of the hardware Align1 region.  The main motivation for this is
>> that currently the lower_integer_multiplication() pass will happily
>> double the stride of one of the 32-bit sources, which can blow up if
>> the stride of the original source was already the maximum value
>> allowed by the hardware.
>
>
> I thought this looked familiar so I did some digging...
>
> On Nov 2 of 2017, I wrote almost exactly this same patch which was committed 
> on Nov 7 as e8c9e65185de3e821e1
> On Nov 14, Matt reverted it in a31d0382084c8aa8 because it wasn't needed 
> anymore and he wasn't sure of its correctness.
>
> And here we are again
>
> I still believe it to be correct so it is
>
> Reviewed-by: Jason Ekstrand 
>
> My one major request is that you include some of the history of this change 
> in the commit message.  As far as the patch itself goes, it's identical to 
> mine except for the unneeded whitespace change and one additional assert 
> which I believe to be a good addition.
>
> I've also CC'd matt in case he wants to throw in his $.02

I don't think I have time to give this a thoughtful review, but as far
as I can remember (with the commit messages to help) the patch I
reverted was the wrong fix at the time. It left some tests failing,
which I fixed in commit 6ac2d1690192, at which point the other commit
was reverted since it was dead code.

Perhaps you've found another case that needs it, but I think it was
the right decision at the time.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/fs: Bail in optimize_extract_to_float if we have modifiers

2019-02-14 Thread Matt Turner
On Mon, Feb 11, 2019 at 8:44 PM Jason Ekstrand  wrote:
>
> This fixes a bug in runscape where we were optimizing x >> 16 to an
> extract and then negating and converting to float.  The NIR to fs pass
> was dropping the negate on the floor breaking a geometry shader and
> causing it to render nothing.
>
> Fixes: 1f862e923cb "i965/fs: Optimize float conversions of byte/word..."
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109601
> Cc: Matt Turner 
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index b80f4351b49..204640ac726 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -510,6 +510,11 @@ fs_visitor::optimize_extract_to_float(nir_alu_instr 
> *instr,
> src0->op != nir_op_extract_i8 && src0->op != nir_op_extract_i16)
>return false;
>
> +   /* If either opcode has source modifiers, bail. */
> +   if (instr->src[0].abs || instr->src[0].negate ||
> +src0->src[0].abs || src0->src[0].negate)

You've done something weird here to vertically align things. I don't
know if I like it, but if you're doing to indent the 'src0' so that
the rest aligns then at least be consistent and align both 'src0's :)

> +  return false;
> +

We could handle this better by allowing the optimization if we're
extracting the high byte/word. In that case I think we could safely
apply the source mod. We should do that separately, but it might be
nice to leave a FINISHME here so we'll remember.

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

Re: [Mesa-dev] [PATCH 1/6] gallium\auxiliary\vl: Move dirty define to header file

2019-02-01 Thread Matt Turner
My OCD is really bothered by the backslashes in the commit title. Can
we use forward slashes like all the other commits?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-announce] [ANNOUNCE] Mesa 18.3.3 release candidate

2019-01-30 Thread Matt Turner
On Wed, Jan 30, 2019 at 10:37 AM Emil Velikov  wrote:
>
> Hello list,
>
> The candidate for the Mesa 18.3.3 is now available. Currently we have:
>  - 45 queued
>  - 4 nominated (outstanding)
>  - and 3 rejected patches

Might be good to cherry-pick the two patches mentioned in
https://bugs.freedesktop.org/show_bug.cgi?id=109488 to 18.3.3 since
they fix a shadertoy shader. I would have Cc'd to stable the one I
committed if I had known it fixed an existing user-visible bug.

They are 76c27e47b90647df047e785d6b3ab5d0d979a1ee ("glsl: Copy
function out to temp if we don't directly ref a variable") and
0862929bf64222e85e8242824aecf05e494c157c ("glsl: Fix copying
function's out to temp if dereferenced by array")
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] nir: Optimize double-precision lower_round_even()

2019-01-28 Thread Matt Turner
On Mon, Jan 28, 2019 at 10:25 AM Roland Scheidegger  wrote:
>
> I like it :-).
> That said, there's some caveats as discussed on IRC - in particular for
> gpus which don't do round-to-nearest-even for ordinary fp64 math (or
> rounding mode could be set to something else manually) it won't do the
> right thing.

I don't know that there are any. Round-to-even is the simplest thing
to do in hardware.

> And if you can have fast-math enabled, then it probably won't round at
> all (at least I think it would be legal to eliminate the add/sub in this
> case).
> So I'm not entirely sure anymore if this can be used unconditionally.
> But I can't really tell if those potential caveats actually matter, hence
> Reviewed-by: Roland Scheidegger 


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


[Mesa-dev] [PATCH] nir: Optimize double-precision lower_round_even()

2019-01-28 Thread Matt Turner
Use the trick of adding and then subtracting 2**52 (52 is the number of
explicit mantissa bits a double-precision floating-point value has) to
implement round-to-even.

Cuts the number of instructions on SKL of the piglit test
fs-roundEven-double.shader_test from 109 to 21.
---
 src/compiler/nir/nir_lower_double_ops.c | 56 ++---
 1 file changed, 12 insertions(+), 44 deletions(-)

diff --git a/src/compiler/nir/nir_lower_double_ops.c 
b/src/compiler/nir/nir_lower_double_ops.c
index 4d4cdf635ea..054fce9c168 100644
--- a/src/compiler/nir/nir_lower_double_ops.c
+++ b/src/compiler/nir/nir_lower_double_ops.c
@@ -392,50 +392,18 @@ lower_fract(nir_builder *b, nir_ssa_def *src)
 static nir_ssa_def *
 lower_round_even(nir_builder *b, nir_ssa_def *src)
 {
-   /* If fract(src) == 0.5, then we will have to decide the rounding direction.
-* We will do this by computing the mod(abs(src), 2) and testing if it
-* is < 1 or not.
-*
-* We compute mod(abs(src), 2) as:
-* abs(src) - 2.0 * floor(abs(src) / 2.0)
-*/
-   nir_ssa_def *two = nir_imm_double(b, 2.0);
-   nir_ssa_def *abs_src = nir_fabs(b, src);
-   nir_ssa_def *mod =
-  nir_fsub(b,
-   abs_src,
-   nir_fmul(b,
-two,
-nir_ffloor(b,
-   nir_fmul(b,
-abs_src,
-nir_imm_double(b, 0.5);
-
-   /*
-* If fract(src) != 0.5, then we round as floor(src + 0.5)
-*
-* If fract(src) == 0.5, then we have to check the modulo:
-*
-*   if it is < 1 we need a trunc operation so we get:
-*  0.5 -> 0,   -0.5 -> -0
-*  2.5 -> 2,   -2.5 -> -2
-*
-*   otherwise we need to check if src >= 0, in which case we need to round
-*   upwards, or not, in which case we need to round downwards so we get:
-*  1.5 -> 2,   -1.5 -> -2
-*  3.5 -> 4,   -3.5 -> -4
-*/
-   nir_ssa_def *fract = nir_ffract(b, src);
-   return nir_bcsel(b,
-nir_fne(b, fract, nir_imm_double(b, 0.5)),
-nir_ffloor(b, nir_fadd(b, src, nir_imm_double(b, 0.5))),
-nir_bcsel(b,
-  nir_flt(b, mod, nir_imm_double(b, 1.0)),
-  nir_ftrunc(b, src),
-  nir_bcsel(b,
-nir_fge(b, src, nir_imm_double(b, 
0.0)),
-nir_fadd(b, src, nir_imm_double(b, 
0.5)),
-nir_fsub(b, src, nir_imm_double(b, 
0.5);
+   /* Add and subtract 2**52 to round off any fractional bits. */
+   nir_ssa_def *two52 = nir_imm_double(b, (double)(1ull << 52));
+   nir_ssa_def *sign = nir_iand(b, nir_unpack_64_2x32_split_y(b, src),
+nir_imm_int(b, 1ull << 31));
+
+   b->exact = true;
+   nir_ssa_def *res = nir_fsub(b, nir_fadd(b, nir_fabs(b, src), two52), two52);
+   b->exact = false;
+
+   return nir_bcsel(b, nir_flt(b, nir_fabs(b, src), two52),
+nir_pack_64_2x32_split(b, nir_unpack_64_2x32_split_x(b, 
res),
+   nir_ior(b, 
nir_unpack_64_2x32_split_y(b, res), sign)), src);
 }
 
 static nir_ssa_def *
-- 
2.19.2

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


[Mesa-dev] [PATCH] i965: Always compile fp64 funcs when needed

2019-01-25 Thread Matt Turner
Compilation of user-specified shaders with software fp64 works by
compiling on demand an "fp64-funcs" shader implementing various fp64
operations and then linking it into the "user shader".

In

   commit 64b8c86d37ebb1e1d286c69d642d52b7bcf051d3
   Author: Timothy Arceri 
   Date:   Thu Jan 17 17:16:29 2019 +1100

   glsl: be much more aggressive when skipping shader compilation

we changed the behavior of the shader cache to skip compilation earlier
when we get a cache hit.

After the aforementioned commit, compiling a user program using fp64
would store into the cache an entry for the fp64-funcs shader.
Subsequent compilations of uncached user shaders using fp64 would fail
in compile_fp64_funcs() after finding a cache entry for the fp64-funcs,
but being unprepared to read from the cache.

It's unclear to me how to retrieve the cached NIR of the fp64-funcs (if
it even is cached), so just call _mesa_glsl_compile_shader() with
force_recompile=true in order to ensure we generate the fp64-funcs
successfully.
---
 src/mesa/drivers/dri/i965/brw_program.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_program.c 
b/src/mesa/drivers/dri/i965/brw_program.c
index c01143decd0..9ab25cf664c 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -40,6 +40,7 @@
 #include "tnl/tnl.h"
 #include "util/ralloc.h"
 #include "compiler/glsl/ir.h"
+#include "compiler/glsl/program.h"
 #include "compiler/glsl/glsl_to_nir.h"
 #include "compiler/glsl/float64_glsl.h"
 
@@ -87,7 +88,7 @@ compile_fp64_funcs(struct gl_context *ctx,
 
sh->Source = float64_source;
sh->CompileStatus = COMPILE_FAILURE;
-   _mesa_compile_shader(ctx, sh);
+   _mesa_glsl_compile_shader(ctx, sh, false, false, true);
 
if (!sh->CompileStatus) {
   if (sh->InfoLog) {
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH] intel/compiler: Add a file-level description of brw_eu_validate.c

2019-01-24 Thread Matt Turner
On Thu, Jan 24, 2019 at 12:16 PM Francisco Jerez  wrote:
>
> Matt Turner  writes:
>
> > ---
> >  src/intel/compiler/brw_eu_validate.c | 14 +-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/intel/compiler/brw_eu_validate.c 
> > b/src/intel/compiler/brw_eu_validate.c
> > index a25010b225c..7f1580a5bb3 100644
> > --- a/src/intel/compiler/brw_eu_validate.c
> > +++ b/src/intel/compiler/brw_eu_validate.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright © 2015 Intel Corporation
> > + * Copyright © 2015-2019 Intel Corporation
> >   *
> >   * Permission is hereby granted, free of charge, to any person obtaining a
> >   * copy of this software and associated documentation files (the 
> > "Software"),
> > @@ -24,6 +24,18 @@
> >  /** @file brw_eu_validate.c
> >   *
> >   * This file implements a pass that validates shader assembly.
> > + *
> > + * The restrictions implemented herein are intended to verify that 
> > instructions
> > + * in shader assembly do not violate restrictions documented in the 
> > graphics
> > + * programming reference manuals.
> > + *
> > + * The restrictions are difficult for humans to quickly verify due to their
> > + * complexity and abundance.
> > + *
> > + * It is critical that this code is thoroughly unit tested because false
> > + * results it will lead developers astray, which is worse than having no
>
> Redundant "it".
>
> > + * validator at all. Patches to this file without corresponding unit tests 
> > (in
> > + * test_eu_validate.cpp) will be rejected.
>
> Strictly by that rule this patch should be rejected ;).  Maybe say
> "functional changes" instead of "patches"?  Other than that:
>
> Reviewed-by: Francisco Jerez 

Fair point :)

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


[Mesa-dev] [PATCH] intel/compiler: Add a file-level description of brw_eu_validate.c

2019-01-24 Thread Matt Turner
---
 src/intel/compiler/brw_eu_validate.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/intel/compiler/brw_eu_validate.c 
b/src/intel/compiler/brw_eu_validate.c
index a25010b225c..7f1580a5bb3 100644
--- a/src/intel/compiler/brw_eu_validate.c
+++ b/src/intel/compiler/brw_eu_validate.c
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2015 Intel Corporation
+ * Copyright © 2015-2019 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -24,6 +24,18 @@
 /** @file brw_eu_validate.c
  *
  * This file implements a pass that validates shader assembly.
+ *
+ * The restrictions implemented herein are intended to verify that instructions
+ * in shader assembly do not violate restrictions documented in the graphics
+ * programming reference manuals.
+ *
+ * The restrictions are difficult for humans to quickly verify due to their
+ * complexity and abundance.
+ *
+ * It is critical that this code is thoroughly unit tested because false
+ * results it will lead developers astray, which is worse than having no
+ * validator at all. Patches to this file without corresponding unit tests (in
+ * test_eu_validate.cpp) will be rejected.
  */
 
 #include "brw_eu.h"
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH] intel/compiler: update validator to account for half-float exec type promotion

2019-01-24 Thread Matt Turner
On Wed, Jan 23, 2019 at 6:03 AM Francisco Jerez  wrote:
>
> Iago Toral Quiroga  writes:
>
> > Commit c84ec70b3a72 implemented execution type promotion to 32-bit for
> > conversions involving half-float registers, which empirical testing 
> > suggested
> > was required, but it did not incorporate this change into the assembly 
> > validator
> > logic. This commits adds that, preventing validation errors like this:
> >
>
> I don't think we should be validating empirical assumptions in the EU
> validator.

I kind of agree. I don't really know what we should do though.

I guess it's better to err on the side of caution in the EU validator
and only check restrictions that have documentation. Is that your
thinking?

Many instructions can only take certain conditional modifiers. XOR is
documented to only take .z/.nz. However we emit XOR with a .l
conditional mod for nir_op_imod. It works, and we think the
documentation is incomplete. Separately it describes how the
conditional modifiers operate, and .l only reads the high bit of the
result so it makes sense that XOR with .l should work like we see that
it does.

So, (1) empirically it works, (2) the documentation says it's not
allowed, but (3) there's a plausible explanation that the
documentation is wrong.

What should we do if we implement the conditional modifier checks in
the validator?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel/compiler: update validator to account for half-float exec type promotion

2019-01-24 Thread Matt Turner
On Wed, Jan 23, 2019 at 4:18 AM Iago Toral Quiroga  wrote:
>
> Commit c84ec70b3a72 implemented execution type promotion to 32-bit for
> conversions involving half-float registers, which empirical testing suggested
> was required, but it did not incorporate this change into the assembly 
> validator
> logic. This commits adds that, preventing validation errors like this:
>
> mov(16)  g9<4>B   g3<16,8,2>HF { align1 1H };
> ERROR: Destination stride must be equal to the ratio of the sizes of the
>execution data type to the destination type
>
> Fixes: c84ec70b3a72 "intel/fs: Promote execution type to 32-bit when any 
> half-float conversion is needed."
> ---
>  src/intel/compiler/brw_eu_validate.c | 27 ++-

New rule: New restrictions (or relaxations) may not be added to
brw_eu_validate.c without accompanying unit tests. I'll send a patch
to add a comment to brw_eu_validate.c saying as much.

Rationale: the reason I wrote brw_eu_validate.c was because I wasted a
week debugging an issue where fulsim not only failed to inform me that
one instruction was invalid but also incorrectly told me that one
correct instruction *was* invalid. I would have been better off
without such a tool.

If the EU validator loses people's trust, then it's useless, but if it
is incorrect it's worse than useless.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v2 1/2] gallium: Enable ASIMD/NEON on aarch64.

2019-01-23 Thread Matt Turner
NEON (now called ASIMD) is available on all aarch64 CPUs. Our code was
missing an aarch64 path, leading to util_cpu_caps.has_neon always being
false on aarch64.
---
Here's the simpler patch to just always enable NEON on aarch64. It suits
my purposes, but I can imagine that you may prefer the original patch if
you ever want to do runtime detection of other features on aarch64.

 src/util/u_cpu_detect.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/util/u_cpu_detect.c b/src/util/u_cpu_detect.c
index 52b9ae547d4..4df10c62ef5 100644
--- a/src/util/u_cpu_detect.c
+++ b/src/util/u_cpu_detect.c
@@ -365,7 +365,14 @@ check_os_arm_support(void)
 }
 #endif /* PIPE_OS_LINUX */
 }
-#endif /* PIPE_ARCH_ARM */
+
+#elif defined(PIPE_ARCH_AARCH64)
+static void
+check_os_arm_support(void)
+{
+util_cpu_caps.has_neon = true;
+}
+#endif /* PIPE_ARCH_ARM || PIPE_ARCH_AARCH64 */
 
 static void
 get_cpu_topology(void)
@@ -534,7 +541,7 @@ util_cpu_detect_once(void)
}
 #endif /* PIPE_ARCH_X86 || PIPE_ARCH_X86_64 */
 
-#if defined(PIPE_ARCH_ARM)
+#if defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
check_os_arm_support();
 #endif
 
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH 1/2] gallium: Enable aarch64 NEON CPU detection.

2019-01-23 Thread Matt Turner
On Wed, Jan 23, 2019 at 3:26 PM Eric Anholt  wrote:
>
> Matt Turner  writes:
>
> > NEON (now called ASIMD) is available on all aarch64 CPUs. It seems that
> > our code was missing an aarch64 path, leading to util_cpu_caps.has_neon
> > always being false on aarch64. I think that means that the NEON tiling
> > code in vc4 would not be enabled on aarch64 (vc4_load_lt_image_neon,
> > etc).
> > ---
> > I have very little clue about aarch64 ABIs, so I don't know if there's
> > another case that needs to be handled -- aarch32 maybe? Does
> > PIPE_ARCH_AARCH64 just mean ARMv8 and so we should check something else
> > for the ABI and choose Elf{32,64} based on that?
>
> Do we actually need to do runtime detection?  It sounds like "standard"
> armv8 is guaranteed NEON:
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0024a/CEGDJGGC.html

I think that's right.
https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html seems to
agree:

‘simd’

Enable Advanced SIMD instructions. This also enables floating-point
instructions. This is on by default for all possible values for
options -march and -mcpu.

I'll send a new patch that just sets util_cpu_caps.has_neon = true for aarch64.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/9] intel/compiler: relax brw_eu_validate for byte raw movs

2019-01-22 Thread Matt Turner
On Tue, Jan 22, 2019 at 10:26 PM Matt Turner  wrote:
> Was this just something that you noticed by inspection?

With the patch reverted I see some validation failures in
dEQP-VK.spirv_assembly.instruction.compute.8bit_storage.push_constant_8_to_16.scalar_uint
and friends.

mov(16) g10<4>B g[a0]B { align1 1H };
ERROR: Destination stride must be equal to the ratio of the sizes of
the execution data type to the destination type

Wow. I don't think I've ever seen an instruction like that before. I
can definitely see how the validator might not be equipped to handle
it.

I'll think about it some more.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/9] intel/compiler: relax brw_eu_validate for byte raw movs

2019-01-22 Thread Matt Turner
On Sun, Jul 8, 2018 at 5:27 PM, Jose Maria Casanova Crespo
 wrote:
> When the destination is a BYTE type allow raw movs
> even if the stride is not exact multiple of destination
> type and exec type, execution type is Word and its size is 2.
>
> This restriction was only allowing stride==2 destinations
> for 8-bit types.

Super late review, obviously... it's been on my todo list but fp64 was
taking all my time.

I can't figure this commit out. What I know:

 - byte destination
 - raw mov (which means destination stride == 1)
 - execution type of a byte operation is "word"

The original code

> if (exec_type_size > dst_type_size) {
>ERROR_IF(dst_stride * dst_type_size != exec_type_size,
> "Destination stride must be equal to the ratio of the sizes 
> of "
> "the execution data type to the destination type");
> }

would not have worked for an instruction like

> mov(8)  g0<1>B  g0<8,8,1>B

But, that's okay because it didn't need to since the block right above
it does this:

   if (dst_type_is_byte) {
  if (is_packed(exec_size * dst_stride, exec_size, dst_stride)) {
 if (!inst_is_raw_move(devinfo, inst)) {
ERROR("Only raw MOV supports a packed-byte destination");
return error_msg;
 } else {
return (struct string){};
 }
  }
   }

That is, if it's a raw move, return no-error.

It would be easier to understand what you were fixing if you had added
a unit test to test_eu_validate.cpp, or (if my suspicions are correct)
it would have proven to you that this patch wasn't correct.

Was this just something that you noticed by inspection?

> Reviewed-by: Jason Ekstrand 

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


Re: [Mesa-dev] [PATCH v3 24/42] intel/compiler: fix ddy for half-float in gen8

2019-01-22 Thread Matt Turner
On Tue, Jan 15, 2019 at 5:54 AM Iago Toral Quiroga  wrote:
>
> We use ALign16 mode for this, since it is more convenient, but the PRM
> for Broadwell states in Volume 3D Media GPGPU, Chapter 'Register region
> restrictions', Section '1. Special Restrictions':
>
>"In Align16 mode, the channel selects and channel enables apply to a
> pair of half-floats, because these parameters are defined for DWord
> elements ONLY. This is applicable when both source and destination
> are half-floats."
>
> This means that we cannot select individual HF elements using swizzles
> like we do with 32-bit floats so we can't implement the required
> regioning for this.
>
> Use the gen11 path for this instead, which uses Align1 mode.
>
> The restriction is not present in gen9 or gen10, where the Align16
> implementation seems to work just fine.
>
> Reviewed-by: Jason Ekstrand 
> ---
>  src/intel/compiler/brw_fs_generator.cpp | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_generator.cpp 
> b/src/intel/compiler/brw_fs_generator.cpp
> index d0cc4a6d231..4310f0b7fdc 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -1339,8 +1339,14 @@ fs_generator::generate_ddy(const fs_inst *inst,
> const uint32_t type_size = type_sz(src.type);
>
> if (inst->opcode == FS_OPCODE_DDY_FINE) {
> -  /* produce accurate derivatives */
> -  if (devinfo->gen >= 11) {
> +  /* produce accurate derivatives. We can do this easily in Align16
> +   * but this is not supported in gen11+ and gen8 Align16 swizzles
> +   * for Half-Float operands work in units of 32-bit and always
> +   * select pairs of consecutive half-float elements, so we can't use
> +   * use it for this.
> +   */

Let's break this comment up and include (or move) the BSpec text from
the commit message here. I wouldn't mention the "this is not supported
in gen11+" because it's slightly unclear whether you're talking about
"accurate derivatives" or "Align16". How about:


  /* produce accurate derivatives.
   *
   * From the Broadwell PRM, Volume 7 (3D-Media-GPGPU)
   * "Register Region Restrictions", Section "1. Special Restrictions":
   *
   *"In Align16 mode, the channel selects and channel enables apply to
   * a pair of half-floats, because these parameters are defined for
   * DWord elements ONLY. This is applicable when both source and
   * destination are half-floats."
   *
   * So for half-float operations we use the Gen11+ Align1 path. CHV
   * inherits its FP16 hardware from SKL, so it is not affected.
   */


> +  if (devinfo->gen >= 11 ||
> +  (devinfo->gen == 8 && src.type == BRW_REGISTER_TYPE_HF)) {


The docs are bad about telling us whether a BDW restriction applies to
CHV as well, but in this case I suspect the answer is no. CHV seems to
inherit its FP16 hw from SKL, which doesn't have the restriction as
you say.

So I suspect you want devinfo->is_broadwell instead of devinfo->gen == 8.

With that (and confirmation that CHV isn't affected), this patch is

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


Re: [Mesa-dev] [PATCH v3 23/42] intel/compiler: fix ddx and ddy for 16-bit float

2019-01-22 Thread Matt Turner
Obviously you cannot test the Gen11 code, but it looks believable.

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


Re: [Mesa-dev] [PATCH v3 22/42] intel/compiler: don't propagate HF immediates to 3-src instructions

2019-01-22 Thread Matt Turner
On Tue, Jan 15, 2019 at 5:54 AM Iago Toral Quiroga  wrote:
>
> 3-src instructions don't support immediates, but since 36bc5f06dd22,
> we allow them on MAD and LRP relying on the combine constants pass to
> fix it up later. However, that pass is specialized for 32-bit float
> immediates and can't handle HF constants at present, so this patch
> ensures that copy-propagation only does this for 32-bit constants.

There's a patch later in the series that adds HF support to constant
combining (and presumably removes this code). Maybe it's the best
thing to add and remove the code in the same series, but it's good to
at least mention that in the commit message so that reviewers
understand what the plan is.

I see from a later thread that you're going to try to handle more than
just F/HF types in constant combining, and I guess you'll resend this
series. If that's the case, I'd just leave this patch out if it's
possible to reorder things.

Oh, another thing: Gen10+ can take *1* immediate HF argument in 3-src
instructions. We haven't added that support yet, though all the
low-level brw_inst_* functions exist. Not asking you to do that
without hardware, but just thought you'd be interested to know :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 21/42] intel/compiler: set correct precision fields for 3-source float instructions

2019-01-22 Thread Matt Turner
On Thu, Jan 17, 2019 at 12:18 PM Jason Ekstrand  wrote:
>
> On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga  wrote:
>>
>> Source0 and Destination extract the floating-point precision automatically
>> from the SrcType and DstType instruction fields respectively when they are
>> set to types :F or :HF. For Source1 and Source2 operands, we use the new
>> 1-bit fields Src1Type and Src2Type, where 0 means normal precision and 1
>> means half-precision. Since we always use the type of the destination for
>> all operands when we emit 3-source instructions, we only need set Src1Type
>> and Src2Type to 1 when we are emitting a half-precision instruction.
>>
>> v2:
>>  - Set the bit separately for each source based on its type so we can
>>do mixed floating-point mode in the future (Topi).
>>
>> Reviewed-by: Topi Pohjolainen 
>> ---
>>  src/intel/compiler/brw_eu_emit.c | 16 
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_eu_emit.c 
>> b/src/intel/compiler/brw_eu_emit.c
>> index a785f96b650..2fa89f8a2a3 100644
>> --- a/src/intel/compiler/brw_eu_emit.c
>> +++ b/src/intel/compiler/brw_eu_emit.c
>> @@ -801,6 +801,22 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct 
>> brw_reg dest,
>>*/
>>   brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);
>>   brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);
>> +
>> + /* From the Bspec: Instruction types
>> +  *
>> +  * Three source instructions can use operands with mixed-mode
>> +  * precision. When SrcType field is set to :f or :hf it defines
>> +  * precision for source 0 only, and fields Src1Type and Src2Type
>> +  * define precision for other source operands:
>> +  *
>> +  *   0b = :f. Single precision Float (32-bit).
>> +  *   1b = :hf. Half precision Float (16-bit).
>> +  */

I'd put this in our typical block-quote style.

>> + if (src1.type == BRW_REGISTER_TYPE_HF)
>> +brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
>
>
> Maybe worth throwing in an
>
> assert(src0.type == BRW_REGISTER_TYPE_F || src0.type == BRW_REGISTER_TYPE_HF);

Please don't. Stuff like this should go in brw_eu_validate.c. If you
add asserts to the emission code, it'll assert fail when you write a
negative test for brw_eu_validate.c.

The code as-is is

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


Re: [Mesa-dev] [PATCH v3 20/42] intel/compiler: allow half-float on 3-source instructions since gen8

2019-01-22 Thread Matt Turner
Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits

2019-01-22 Thread Matt Turner
On Tue, Jan 15, 2019 at 5:55 AM Iago Toral Quiroga  wrote:
>
> We are now using these bits, so don't assert that they are not set, just
> avoid compaction in that case.
>
> Reviewed-by: Topi Pohjolainen 
> ---
>  src/intel/compiler/brw_eu_compact.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_eu_compact.c 
> b/src/intel/compiler/brw_eu_compact.c
> index ae14ef10ec0..20fed254331 100644
> --- a/src/intel/compiler/brw_eu_compact.c
> +++ b/src/intel/compiler/brw_eu_compact.c
> @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info 
> *devinfo,
>assert(!brw_inst_bits(src, 127, 126) &&
>   !brw_inst_bits(src, 105, 105) &&
>   !brw_inst_bits(src, 84, 84) &&
> - !brw_inst_bits(src, 36, 35) &&
>   !brw_inst_bits(src, 7,  7));
> +
> +  /* Src1Type and Src2Type, used for mixed-precision floating point */
> +  if (brw_inst_bits(src, 36, 35))
> + return true;
> }


These bits are used on SKL+ and CHV (which is handled immediately
above this hunk), so this is only modifying BDW. All looks correct to
me.

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


Re: [Mesa-dev] [PATCH v3 17/42] intel/compiler: add new half-float register type for 3-src instructions

2019-01-22 Thread Matt Turner
On Tue, Jan 15, 2019 at 5:55 AM Iago Toral Quiroga  wrote:
>
> This is available since gen8.
>
> v2: restore previously existing assertion.
>
> Reviewed-by: Topi Pohjolainen  (v1)
> ---
>  src/intel/compiler/brw_reg_type.c | 36 +++
>  1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/compiler/brw_reg_type.c 
> b/src/intel/compiler/brw_reg_type.c
> index 60240ba1513..09b3ea61d4c 100644
> --- a/src/intel/compiler/brw_reg_type.c
> +++ b/src/intel/compiler/brw_reg_type.c
> @@ -138,6 +138,7 @@ enum hw_3src_reg_type {
> GEN7_3SRC_TYPE_D  = 1,
> GEN7_3SRC_TYPE_UD = 2,
> GEN7_3SRC_TYPE_DF = 3,
> +   GEN8_3SRC_TYPE_HF = 4,
>
> /** When ExecutionDatatype is 1: @{ */
> GEN10_ALIGN1_3SRC_REG_TYPE_HF = 0b000,
> @@ -166,6 +167,14 @@ static const struct hw_3src_type {
> [BRW_REGISTER_TYPE_D]  = { GEN7_3SRC_TYPE_D  },
> [BRW_REGISTER_TYPE_UD] = { GEN7_3SRC_TYPE_UD },
> [BRW_REGISTER_TYPE_DF] = { GEN7_3SRC_TYPE_DF },
> +}, gen8_hw_3src_type[] = {
> +   [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID },
> +
> +   [BRW_REGISTER_TYPE_F]  = { GEN7_3SRC_TYPE_F  },
> +   [BRW_REGISTER_TYPE_D]  = { GEN7_3SRC_TYPE_D  },
> +   [BRW_REGISTER_TYPE_UD] = { GEN7_3SRC_TYPE_UD },
> +   [BRW_REGISTER_TYPE_DF] = { GEN7_3SRC_TYPE_DF },
> +   [BRW_REGISTER_TYPE_HF] = { GEN8_3SRC_TYPE_HF },
>  }, gen10_hw_3src_align1_type[] = {
>  #define E(x) BRW_ALIGN1_3SRC_EXEC_TYPE_##x
> [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID },
> @@ -249,6 +258,20 @@ brw_hw_type_to_reg_type(const struct gen_device_info 
> *devinfo,
> unreachable("not reached");
>  }
>
> +static inline const struct hw_3src_type *
> +get_hw_3src_type_map(const struct gen_device_info *devinfo, uint32_t *size)
> +{
> +   if (devinfo->gen < 8) {
> +  if (size)
> + *size = ARRAY_SIZE(gen7_hw_3src_type);
> +  return gen7_hw_3src_type;
> +   } else {
> +  if (size)
> + *size = ARRAY_SIZE(gen8_hw_3src_type);
> +  return gen8_hw_3src_type;
> +   }
> +}

I would rather inline this code and remove the function, like we
already do for example:

   const struct hw_type *table;

   if (devinfo->gen >= 11) {
  assert(type < ARRAY_SIZE(gen11_hw_type));
  table = gen11_hw_type;
   } else {
  assert(type < ARRAY_SIZE(gen4_hw_type));
  table = gen4_hw_type;
   }

But I'm not even sure that separate gen7 vs gen8 tables are required,
since gen8 just adds one additional value. I thought we had some code
that essentially did assert(devinfo->gen >= 8 || type !=
BRW_REGISTER_TYPE_HF), but I don't see it now.

We have checks that Q/UQ/DF are only used when 64-bit hw support is
available, so maybe that's what I'm thinking of.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel/compiler: Reset default flag register in brw_find_live_channel()

2019-01-22 Thread Matt Turner
On Tue, Jan 22, 2019 at 3:25 PM Francisco Jerez  wrote:
>
> Matt Turner  writes:
>
> > emit_uniformize() emits SHADER_OPCODE_FIND_LIVE_CHANNEL with its
> > flag_subreg set, so that the IR knows which flag is accessed. However
> > the flag is only used on Gen7 in Align1 mode.
> >
> > To avoid setting unnecessary bits in the instruction words, get the
> > information we need and reset the default flag register. This allows
> > round-tripping through the assembler/disassembler.
> > ---
> >  src/intel/compiler/brw_eu_emit.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_eu_emit.c 
> > b/src/intel/compiler/brw_eu_emit.c
> > index 45e2552783b..7c5b40af3ae 100644
> > --- a/src/intel/compiler/brw_eu_emit.c
> > +++ b/src/intel/compiler/brw_eu_emit.c
> > @@ -3312,6 +3312,13 @@ brw_find_live_channel(struct brw_codegen *p, struct 
> > brw_reg dst,
> >
> > brw_push_insn_state(p);
> >
> > +   /* The flag register is only used on Gen7 in align1 mode, so avoid 
> > setting
> > +* unnecessary bits in the instruction words, get the information we 
> > need
> > +* and reset the default flag register.
>
> Maybe mention here that this also allows more instructions to be
> compacted.  Looks good otherwise:
>
> Reviewed-by: Francisco Jerez 

Sure, will do. Thanks Curro!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] intel/compiler: Reset default flag register in brw_find_live_channel()

2019-01-22 Thread Matt Turner
emit_uniformize() emits SHADER_OPCODE_FIND_LIVE_CHANNEL with its
flag_subreg set, so that the IR knows which flag is accessed. However
the flag is only used on Gen7 in Align1 mode.

To avoid setting unnecessary bits in the instruction words, get the
information we need and reset the default flag register. This allows
round-tripping through the assembler/disassembler.
---
 src/intel/compiler/brw_eu_emit.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
index 45e2552783b..7c5b40af3ae 100644
--- a/src/intel/compiler/brw_eu_emit.c
+++ b/src/intel/compiler/brw_eu_emit.c
@@ -3312,6 +3312,13 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
 
brw_push_insn_state(p);
 
+   /* The flag register is only used on Gen7 in align1 mode, so avoid setting
+* unnecessary bits in the instruction words, get the information we need
+* and reset the default flag register.
+*/
+   const unsigned flag_subreg = p->current->flag_subreg;
+   brw_set_default_flag_reg(p, 0, 0);
+
if (brw_get_default_access_mode(p) == BRW_ALIGN_1) {
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
 
@@ -3345,8 +3352,7 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
   */
  inst = brw_FBL(p, vec1(dst), exec_mask);
   } else {
- const struct brw_reg flag = brw_flag_reg(p->current->flag_subreg / 2,
-  p->current->flag_subreg % 2);
+ const struct brw_reg flag = brw_flag_subreg(flag_subreg);
 
  brw_set_default_exec_size(p, BRW_EXECUTE_1);
  brw_MOV(p, retype(flag, BRW_REGISTER_TYPE_UD), brw_imm_ud(0));
@@ -3366,6 +3372,8 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
 brw_inst_set_group(devinfo, inst, lower_size * i + 8 * 
qtr_control);
 brw_inst_set_cond_modifier(devinfo, inst, BRW_CONDITIONAL_Z);
 brw_inst_set_exec_size(devinfo, inst, cvt(lower_size) - 1);
+brw_inst_set_flag_reg_nr(devinfo, inst, flag_subreg / 2);
+brw_inst_set_flag_subreg_nr(devinfo, inst, flag_subreg % 2);
  }
 
  /* Find the first bit set in the exec_size-wide portion of the flag
-- 
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] gallivm: Return true from arch_rounding_available() if NEON is available

2019-01-22 Thread Matt Turner
LLVM uses the single instruction "FRINTI" to implement llvm.nearbyint.
Fixes the rounding tests of lp_test_arit.

Bug: https://bugs.gentoo.org/665570
---
 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 4 +++-
 src/gallium/drivers/llvmpipe/lp_test_arit.c | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index c050bfdb936..057c50ed278 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -1992,6 +1992,8 @@ arch_rounding_available(const struct lp_type type)
else if ((util_cpu_caps.has_altivec &&
 (type.width == 32 && type.length == 4)))
   return TRUE;
+   else if (util_cpu_caps.has_neon)
+  return TRUE;
 
return FALSE;
 }
@@ -2099,7 +2101,7 @@ lp_build_round_arch(struct lp_build_context *bld,
 LLVMValueRef a,
 enum lp_build_round_mode mode)
 {
-   if (util_cpu_caps.has_sse4_1) {
+   if (util_cpu_caps.has_sse4_1 || util_cpu_caps.has_neon) {
   LLVMBuilderRef builder = bld->gallivm->builder;
   const struct lp_type type = bld->type;
   const char *intrinsic_root;
diff --git a/src/gallium/drivers/llvmpipe/lp_test_arit.c 
b/src/gallium/drivers/llvmpipe/lp_test_arit.c
index acba7ed44a8..eb3f67dc1fe 100644
--- a/src/gallium/drivers/llvmpipe/lp_test_arit.c
+++ b/src/gallium/drivers/llvmpipe/lp_test_arit.c
@@ -458,7 +458,8 @@ test_unary(unsigned verbose, FILE *fp, const struct 
unary_test_t *test, unsigned
 continue;
  }
 
- if (test->ref ==  && length == 2 && 
+ if (!util_cpu_caps.has_neon &&
+ test->ref ==  && length == 2 &&
  ref != roundf(testval)) {
 /* FIXME: The generic (non SSE) path in lp_build_iround, which is
  * always taken for length==2 regardless of native round support,
-- 
2.19.2

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


[Mesa-dev] [PATCH 1/2] gallium: Enable aarch64 NEON CPU detection.

2019-01-22 Thread Matt Turner
NEON (now called ASIMD) is available on all aarch64 CPUs. It seems that
our code was missing an aarch64 path, leading to util_cpu_caps.has_neon
always being false on aarch64. I think that means that the NEON tiling
code in vc4 would not be enabled on aarch64 (vc4_load_lt_image_neon,
etc).
---
I have very little clue about aarch64 ABIs, so I don't know if there's
another case that needs to be handled -- aarch32 maybe? Does
PIPE_ARCH_AARCH64 just mean ARMv8 and so we should check something else
for the ABI and choose Elf{32,64} based on that?

Also, Android is not handled.

 src/util/u_cpu_detect.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/util/u_cpu_detect.c b/src/util/u_cpu_detect.c
index 52b9ae547d4..e9cdb78e458 100644
--- a/src/util/u_cpu_detect.c
+++ b/src/util/u_cpu_detect.c
@@ -328,7 +328,7 @@ PIPE_ALIGN_STACK static inline boolean sse2_has_daz(void)
 
 #endif /* X86 or X86_64 */
 
-#if defined(PIPE_ARCH_ARM)
+#if defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
 static void
 check_os_arm_support(void)
 {
@@ -348,24 +348,36 @@ check_os_arm_support(void)
  util_cpu_caps.has_neon = 1;
}
 #elif defined(PIPE_OS_LINUX)
-Elf32_auxv_t aux;
+
+#if defined(PIPE_ARCH_ARM)
+#define Elf_auxv_t Elf32_auxv_t
+#elif defined(PIPE_ARCH_AARCH64)
+#define Elf_auxv_t Elf64_auxv_t
+#endif
+
+Elf_auxv_t aux;
 int fd;
 
 fd = open("/proc/self/auxv", O_RDONLY | O_CLOEXEC);
 if (fd >= 0) {
-   while (read(fd, , sizeof(Elf32_auxv_t)) == sizeof(Elf32_auxv_t)) {
+   while (read(fd, , sizeof(Elf_auxv_t)) == sizeof(Elf_auxv_t)) {
   if (aux.a_type == AT_HWCAP) {
  uint32_t hwcap = aux.a_un.a_val;
 
+#if defined(PIPE_ARCH_ARM)
  util_cpu_caps.has_neon = (hwcap >> 12) & 1;
+#elif defined(PIPE_ARCH_AARCH64)
+ util_cpu_caps.has_neon = (hwcap >> 1) & 1;
+#endif
  break;
   }
}
close (fd);
 }
+#undef Elf_auxv_t
 #endif /* PIPE_OS_LINUX */
 }
-#endif /* PIPE_ARCH_ARM */
+#endif /* PIPE_ARCH_ARM || PIPE_ARCH_AARCH64 */
 
 static void
 get_cpu_topology(void)
@@ -534,7 +546,7 @@ util_cpu_detect_once(void)
}
 #endif /* PIPE_ARCH_X86 || PIPE_ARCH_X86_64 */
 
-#if defined(PIPE_ARCH_ARM)
+#if defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
check_os_arm_support();
 #endif
 
-- 
2.19.2

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


[Mesa-dev] [PATCH] intel/compiler: Reset default flag register in brw_find_live_channel()

2019-01-22 Thread Matt Turner
emit_uniformize() emits SHADER_OPCODE_FIND_LIVE_CHANNEL with its
flag_subreg set, so that the IR knows which flag is accessed. However
the flag is only used on Gen7 in Align1 mode.

To avoid setting unnecessary bits in the instruction words, get the
information we need and reset the default flag register. This allows
round-tripping through the assembler/disassembler.
---
 src/intel/compiler/brw_eu_emit.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
index 45e2552783b..d05ea506353 100644
--- a/src/intel/compiler/brw_eu_emit.c
+++ b/src/intel/compiler/brw_eu_emit.c
@@ -3312,6 +3312,14 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
 
brw_push_insn_state(p);
 
+   /* The flag register is only used on Gen7 in align1 mode, so avoid setting
+* unnecessary bits in the instruction words, get the information we need
+* and reset the default flag register.
+*/
+   int flag_reg = p->current->flag_subreg / 2;
+   int flag_subreg = p->current->flag_subreg % 2;
+   brw_set_default_flag_reg(p, 0, 0);
+
if (brw_get_default_access_mode(p) == BRW_ALIGN_1) {
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
 
@@ -3345,12 +3353,14 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
   */
  inst = brw_FBL(p, vec1(dst), exec_mask);
   } else {
- const struct brw_reg flag = brw_flag_reg(p->current->flag_subreg / 2,
-  p->current->flag_subreg % 2);
+ const struct brw_reg flag = brw_flag_reg(flag_reg, flag_subreg);
 
  brw_set_default_exec_size(p, BRW_EXECUTE_1);
  brw_MOV(p, retype(flag, BRW_REGISTER_TYPE_UD), brw_imm_ud(0));
 
+ brw_push_insn_state(p);
+ brw_set_default_flag_reg(p, flag_reg, flag_subreg);
+
  /* Run enough instructions returning zero with execution masking and
   * a conditional modifier enabled in order to get the full execution
   * mask in f1.0.  We could use a single 32-wide move here if it
@@ -3368,6 +3378,8 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
 brw_inst_set_exec_size(devinfo, inst, cvt(lower_size) - 1);
  }
 
+ brw_pop_insn_state(p);
+
  /* Find the first bit set in the exec_size-wide portion of the flag
   * register that was updated by the last sequence of MOV
   * instructions.
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH] intel/compiler: Reset default flag register in brw_find_live_channel()

2019-01-22 Thread Matt Turner
On Tue, Jan 22, 2019 at 11:53 AM Francisco Jerez  wrote:
>
> Matt Turner  writes:
>
> > emit_uniformize() emits SHADER_OPCODE_FIND_LIVE_CHANNEL with its
> > flag_subreg set, so that the IR knows which flag is accessed. However
> > the flag is only used on Gen7 in Align1 mode, and it is used as an
> > explicit source and destination.
> >
> > To avoid setting unnecessary bits in the instruction words, get the
> > information we need and reset the default flag register. This allows
> > round-tripping through the assembler/disassembler.
> > ---
> >  src/intel/compiler/brw_eu_emit.c | 11 ---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_eu_emit.c 
> > b/src/intel/compiler/brw_eu_emit.c
> > index 45e2552783b..e6f6d6419d2 100644
> > --- a/src/intel/compiler/brw_eu_emit.c
> > +++ b/src/intel/compiler/brw_eu_emit.c
> > @@ -3312,6 +3312,14 @@ brw_find_live_channel(struct brw_codegen *p, struct 
> > brw_reg dst,
> >
> > brw_push_insn_state(p);
> >
> > +   /* The flag register is only used on Gen7 in align1 mode, so avoid 
> > setting
> > +* unnecessary bits in the instruction words, get the information we 
> > need
> > +* and reset the default flag register.
> > +*/
> > +   const struct brw_reg flag = brw_flag_reg(p->current->flag_subreg / 2,
> > +p->current->flag_subreg % 2);
> > +   brw_set_default_flag_reg(p, 0, 0);
> > +
>
> I think this is going to break Gen7, because the MOV instructions
> emitted in the loop below have conditional mod enabled and won't be
> pointing at the right flag register anymore after this change.

Crap, I missed that. I'll send an updated patch.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] intel/compiler: Reset default flag register in brw_find_live_channel()

2019-01-22 Thread Matt Turner
emit_uniformize() emits SHADER_OPCODE_FIND_LIVE_CHANNEL with its
flag_subreg set, so that the IR knows which flag is accessed. However
the flag is only used on Gen7 in Align1 mode, and it is used as an
explicit source and destination.

To avoid setting unnecessary bits in the instruction words, get the
information we need and reset the default flag register. This allows
round-tripping through the assembler/disassembler.
---
 src/intel/compiler/brw_eu_emit.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
index 45e2552783b..e6f6d6419d2 100644
--- a/src/intel/compiler/brw_eu_emit.c
+++ b/src/intel/compiler/brw_eu_emit.c
@@ -3312,6 +3312,14 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
 
brw_push_insn_state(p);
 
+   /* The flag register is only used on Gen7 in align1 mode, so avoid setting
+* unnecessary bits in the instruction words, get the information we need
+* and reset the default flag register.
+*/
+   const struct brw_reg flag = brw_flag_reg(p->current->flag_subreg / 2,
+p->current->flag_subreg % 2);
+   brw_set_default_flag_reg(p, 0, 0);
+
if (brw_get_default_access_mode(p) == BRW_ALIGN_1) {
   brw_set_default_mask_control(p, BRW_MASK_DISABLE);
 
@@ -3345,9 +3353,6 @@ brw_find_live_channel(struct brw_codegen *p, struct 
brw_reg dst,
   */
  inst = brw_FBL(p, vec1(dst), exec_mask);
   } else {
- const struct brw_reg flag = brw_flag_reg(p->current->flag_subreg / 2,
-  p->current->flag_subreg % 2);
-
  brw_set_default_exec_size(p, BRW_EXECUTE_1);
  brw_MOV(p, retype(flag, BRW_REGISTER_TYPE_UD), brw_imm_ud(0));
 
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH v3 16/42] intel/compiler: add instruction setters for Src1Type and Src2Type.

2019-01-22 Thread Matt Turner
Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 39/42] intel/compiler: remove MAD/LRP algebraic optimizations from the backend

2019-01-17 Thread Matt Turner
On Tue, Jan 15, 2019 at 5:54 AM Iago Toral Quiroga  wrote:
>
> NIR already has these so they are redundant. A run of shader-db confirms
> that the only cases where these backend optimizations are activated
> are some Tomb Raider shaders where the affected variables are qualified
> as "precise", which is why NIR won't apply them and why the backend
> shouldn't either (so it is actually a bug).

Which of the six optimizations that you're removing were responsible
for the change? I ask because...

>
> Suggested-by: Jason Ekstrand 
> ---
>  src/intel/compiler/brw_fs.cpp | 37 ---
>  1 file changed, 37 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 77c955ac435..e7f5a8822a3 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -2568,16 +2568,6 @@ fs_visitor::opt_algebraic()
>  break;
>   }
>   break;
> -  case BRW_OPCODE_LRP:
> - if (inst->src[1].equals(inst->src[2])) {
> -inst->opcode = BRW_OPCODE_MOV;
> -inst->src[0] = inst->src[1];
> -inst->src[1] = reg_undef;
> -inst->src[2] = reg_undef;
> -progress = true;
> -break;

I'm not sure whether this is imprecise, and...

> - }
> - break;
>case BRW_OPCODE_CMP:
>   if ((inst->conditional_mod == BRW_CONDITIONAL_Z ||
>inst->conditional_mod == BRW_CONDITIONAL_NZ) &&
> @@ -2654,33 +2644,6 @@ fs_visitor::opt_algebraic()
>  }
>   }
>   break;
> -  case BRW_OPCODE_MAD:
> - if (inst->src[1].is_zero() || inst->src[2].is_zero()) {
> -inst->opcode = BRW_OPCODE_MOV;
> -inst->src[1] = reg_undef;
> -inst->src[2] = reg_undef;
> -progress = true;
> - } else if (inst->src[0].is_zero()) {
> -inst->opcode = BRW_OPCODE_MUL;
> -inst->src[0] = inst->src[2];
> -inst->src[2] = reg_undef;
> -progress = true;
> - } else if (inst->src[1].is_one()) {
> -inst->opcode = BRW_OPCODE_ADD;
> -inst->src[1] = inst->src[2];
> -inst->src[2] = reg_undef;
> -progress = true;
> - } else if (inst->src[2].is_one()) {
> -inst->opcode = BRW_OPCODE_ADD;
> -inst->src[2] = reg_undef;
> -progress = true;
> - } else if (inst->src[1].file == IMM && inst->src[2].file == IMM) {
> -inst->opcode = BRW_OPCODE_ADD;
> -inst->src[1].f *= inst->src[2].f;
> -inst->src[2] = reg_undef;
> -progress = true;

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


Re: [Mesa-dev] [PATCH] intel/fs: Don't apply the des stride alignment rule to accumulators

2019-01-16 Thread Matt Turner
On Wed, Jan 16, 2019 at 8:40 PM Jason Ekstrand  wrote:
>
> The pass was discovered to cause problems with the MUL+MACH combination
> we emit for nir_[iu]mul_high.  In an experimental branch of mine, I ran
> into issues where the MUL+MACH ended up using a strided source due to
> working on half of a uint64_t and the new lowering pass helpfully tried
> to fix the multiply which wrote to an unstriated accumulator.  Not only

unstrided

> did the multiply not need to be fixed but the "fix" ended up breaking it
> because a MOV to the accumulator is not the same as using it as a
> multiply destination due to the magic way the 33/64 bits of the
> accumulator are handled for different instruction types.
>
> Fixes: efa4e4bc5fc "intel/fs: Introduce regioning lowering pass"
> Cc: Francisco Jerez 
> ---
>  src/intel/compiler/brw_fs_lower_regioning.cpp | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/compiler/brw_fs_lower_regioning.cpp 
> b/src/intel/compiler/brw_fs_lower_regioning.cpp
> index cc4163b4c2c..b8a89e82272 100644
> --- a/src/intel/compiler/brw_fs_lower_regioning.cpp
> +++ b/src/intel/compiler/brw_fs_lower_regioning.cpp
> @@ -53,7 +53,13 @@ namespace {
> unsigned
> required_dst_byte_stride(const fs_inst *inst)
> {
> -  if (type_sz(inst->dst.type) < get_exec_type_size(inst) &&
> +  if (inst->dst.is_accumulator()) {
> + /* Even though it's not explicitly documented in the PRMs or the
> +  * BSpec, writes to the accumulator appear to not need any special
> +  * treatment with respect too their destination stride alignment.

to


I read the backlog on IRC and I still don't understand, but that's okay :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] NIR + softfp64 problem

2019-01-16 Thread Matt Turner
On Wed, Jan 16, 2019 at 3:53 AM Gert Wollny  wrote:
>
> Hello,
>
> I'm trying to get soft-fp64 working with my experimental r600-nir
> backend, and thanks to Dave's contribution it is already tied in,
> some instructions are not yet supported by the backend, but when
> running the piglits I have already 24 tests passing. However, there are
> also 1099 test crashing, and skimming over the results this is mostly
> because of validation erros in nir_lower_doubles called with
> nir_lower_fp64_full_software. For instance, in fs-fract-dvec4 I get
> something like
>
> decl_var  INTERP_MODE_NONE dvec4 p
> ...
> vec1 32 ssa_0 = deref_var  (function dvec4)
> ...
> call __fadd64 ssa_77, ssa_78, ssa_79
> vec1 64 ssa_80 = intrinsic load_deref (ssa_77) (0) /* access=0
> */intrinsic store_deref (ssa_0, ssa_80) (15, 0) /* wrmask=xyzw */ /*
> access=0 */
> error: src->ssa->num_components == num_components
> (../../../mesa/src/compiler/nir/nir_validate.c:206)
> ...
>
> with (src->ssa->num_components == 1) and  (num_components == 4).
>
> i.e. a function returns a single value, but the target where to store
> it is a vector with four elements.
>
> I have no idea whether this is something triggered by the compile
> options I defined for nir, or whether this is independed from this, so
> any idea how to quell this would be very welcome.

It's required to scalarize fp64 operations before this lowering code
will work. It looks to me like it's trying to call __fadd64 with a
dvec4 argument, when the arguments are actually scalar.

I think r600 is mostly vector-based? The soft-fp64 code probably isn't
ideally suited for that. I'd attempt to call nir_lower_alu_to_scalar()
before calling nir_lower_doubles(). That should at minimum tell you
whether my hypothesis is correct.

From there, maybe we could pass an options bitfield to
nir_lower_alu_to_scalar() to allow R600 to only lower fp64 operations
and not scalarize everything. Or, we could try to figure out how to
add vectorized versions of the soft-fp64 routines that R600 could use
directly.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2019-01-16 Thread Matt Turner
On Fri, Jan 11, 2019 at 2:11 PM Matt Turner  wrote:
>
> From: Gert Wollny 
>
> Since Meson will eventually be the only build system deprecate autotools
> now. It can still be used by invoking configure with the flag
>   --enable-autotools
>
> NAKed-by: Ilia Mirkin 
> Acked-by: Eric Engestrom 
> Acked-by: Kenneth Graunke 
> Acked-by: Lionel Landwerlin 
> Acked-by: Jason Ekstrand 
> Acked-by: Rob Clark 
> Acked-by: Marek Olšák 
> Reviewed-by: Christian Gmeiner 
> Reviewed-by: Matt Turner 
> Reviewed-by: Eric Anholt 
> Signed-off-by: Gert Wollny 
> ---

Now upstream as commit e68777c87ceed02ab199b32f941778c3cf97c794.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/4] intel/sanitize_gpu: add help/gdb options to wrapper

2019-01-15 Thread Matt Turner
On Mon, Oct 29, 2018 at 11:16 AM Lionel Landwerlin
 wrote:
>
> Signed-off-by: Lionel Landwerlin 
> ---
>  src/intel/tools/intel_sanitize_gpu.in | 55 ++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/tools/intel_sanitize_gpu.in 
> b/src/intel/tools/intel_sanitize_gpu.in
> index 3dac954c408..7e4c96d8738 100755
> --- a/src/intel/tools/intel_sanitize_gpu.in
> +++ b/src/intel/tools/intel_sanitize_gpu.in
> @@ -1,4 +1,57 @@
>  #!/bin/bash
>  # -*- mode: sh -*-
>
> -LD_PRELOAD="@install_libexecdir@/libintel_sanitize_gpu.so${LD_PRELOAD:+:$LD_PRELOAD}"
>  exec "$@"
> +function show_help() {
> +cat < +Usage: intel_sanitize_gpu [OPTION]... [--] COMMAND ARGUMENTS
> +
> +Run COMMAND with ARGUMENTS and verify the GPU doesn't write outside its 
> memory
> +mapped buffers.
> +
> +  -g, --gdb  Launch GDB
> +
> +  --help Display this help message and exit
> +
> +EOF
> +
> +exit 0
> +}
> +
> +gdb=""
> +
> +while true; do
> +case "$1" in
> +--gdb)
> +gdb=1
> +shift
> +;;
> +-g)
> +gdb=1
> +shift
> +;;
> +--help)
> +show_help
> +;;
> +--)
> +shift
> +break
> +;;
> +-*)
> +echo "intel_aubdump: invalid option: $1"

No idea why this patch never landed, but

s/intel_aubdump/intel_sanitize_gpu/

(I just came across it when trying to figure out whether we ever moved
intel_aubdump from igt into Mesa.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] intel/fs: Do the grf127 hack on SIMD8 instructions in SIMD16 mode

2019-01-15 Thread Matt Turner
On Tue, Jan 15, 2019 at 8:58 AM Jason Ekstrand  wrote:
>
> Previously, we only applied the fix to shaders with a dispatch mode of
> SIMD8 but the code it relies on for SIMD16 mode only applies to SIMD16
> instructions.  If you have a SIMD8 instruction in a SIMD16 shader,
> neither would trigger and the restriction could still be hit.
>
> Cc: Jose Maria Casanova Crespo 
> Fixes: 232ed8980217dd "i965/fs: Register allocator shoudn't use grf127..."
> ---
>  src/intel/compiler/brw_fs_reg_allocate.cpp | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_reg_allocate.cpp 
> b/src/intel/compiler/brw_fs_reg_allocate.cpp
> index 5db5242452e..ec743f9b5bf 100644
> --- a/src/intel/compiler/brw_fs_reg_allocate.cpp.
> +++ b/src/intel/compiler/brw_fs_reg_allocate.cpp
> @@ -667,15 +667,14 @@ fs_visitor::assign_regs(bool allow_spilling, bool 
> spill_all)
> * messages adding a node interference to the grf127_send_hack_node.
> * This node has a fixed asignment to grf127.
> *
> -   * We don't apply it to SIMD16 because previous code avoids any 
> register
> -   * overlap between sources and destination.
> +   * We don't apply it to SIMD16 instructions because previous code 
> avoids
> +   * any register overlap between sources and destination.
> */
>ra_set_node_reg(g, grf127_send_hack_node, 127);
> -  if (dispatch_width == 8) {
> - foreach_block_and_inst(block, fs_inst, inst, cfg) {
> -if (inst->is_send_from_grf() && inst->dst.file == VGRF)
> -   ra_add_node_interference(g, inst->dst.nr, 
> grf127_send_hack_node);
> - }
> +  foreach_block_and_inst(block, fs_inst, inst, cfg) {
> + if (inst->exec_size < 16 && inst->is_send_from_grf() &&
> + inst->dst.file == VGRF)
> +ra_add_node_interference(g, inst->dst.nr, grf127_send_hack_node);
>}
>

Did the code in brw_eu_validate.c catch the case you found?

In fact, that code looks wrong:

|  (brw_inst_dst_da_reg_nr(devinfo, inst) +
|   brw_inst_rlen(devinfo, inst) > 127) &&

I think > should be >=. And maybe we should have a separate case
earlier that checks that dst_nr+rlen actually fits in registers, and
then change > to just ==. FFS :(

Not sure what I was thinking letting that patch through without a unit
test. I'll do that.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-15 Thread Matt Turner
On Mon, Jan 14, 2019 at 4:36 AM Daniel Stone  wrote:
>
> Hi,
>
> On Fri, 11 Jan 2019 at 17:05, Jason Ekstrand  wrote:
> >  5. There's no way with gitlab for Reviewed-by tags to get automatically 
> > applied as part of the merging process.  This makes merging a bit more 
> > manual than it needs to be but is really no worse than it was before.
>
> I'm still on the side of not seeing the value in them.

Reviewed-by tags are useful for measuring the quantity of patch review
people do (which is useful in a corporate environment...). It's often
a thankless task that's valued much lower than first order
contributions, so having a way to at least quantify patch reviews
shows that people are spending their time to help others contribute.

The number of R-b tags is not a 100% accurate picture of the
situation, but it gives at least a good overview of who is doing the
tedious work of patch review. For instance, in 2018 the top reviewers
are

620 Bas Nieuwenhuizen 
530 Marek Olšák 
505 Jason Ekstrand 
452 Kenneth Graunke 

If my name were in there, it would definitely be something I put on my
yearly review.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2019-01-11 Thread Matt Turner
On Fri, Jan 11, 2019 at 2:28 PM Ilia Mirkin  wrote:
>
> On Fri, Jan 11, 2019 at 5:12 PM Matt Turner  wrote:
> >
> > From: Gert Wollny 
> >
> > Since Meson will eventually be the only build system deprecate autotools
> > now. It can still be used by invoking configure with the flag
> >   --enable-autotools
> >
> > NAKed-by: Ilia Mirkin 
>
> [nouveau]
>
> > Acked-by: Eric Engestrom 
> > Acked-by: Kenneth Graunke 
> > Acked-by: Lionel Landwerlin 
> > Acked-by: Jason Ekstrand 
> > Reviewed-by: Matt Turner 
>
> [intel]
>
> > Acked-by: Rob Clark 
>
> [freedreno]
>
> > Acked-by: Marek Olšák 
>
> [radeon]
>
> > Reviewed-by: Christian Gmeiner 
>
> [etnaviv]
>
> > Reviewed-by: Eric Anholt 
>
> [vc4]
>
> > Signed-off-by: Gert Wollny 
>
> [sorry Gert, not sure how to classify you]
>
> I think the vmware team (which largely maintains llvmpipe and svga) is
> probably worth hearing from -- I believe they've largely stayed out of
> it. But an ack/nack would be good. Also virgl isn't represented, I
> believe. Probably not *required* to hear from these, but perhaps worth
> a poke?

Sure. I've Cc'd Dave, Brian, José, and Roland on this reply.

> > ---
> > I think there's support for overriding the sole objection to this patch.
> >
> > To confirm:
> >
> > (1) The plan is to remove Autotools, perhaps after the 19.0 release
> >
> > (2) This patch's purpose is to ensure that everyone knows that
> > Autotools will be going away (think: people who build Mesa as
> > part of an automated process and wouldn't notice a deprecation
> > warning unless it requires some action from them)
>
> If it's being removed _after_ the 19.0 release, does it make sense to
> have a patch like this _in_ the 19.0 release? (Perhaps the answer is
> `yes', but I'd still like to ask the question.)

Yes, I think so -- I might be missing or misunderstanding a part of
your question though.

My thinking is in 19.0 mark autotools as deprecated with this patch so
as to ensure everyone knows, and depending on progress on the blocking
issues to aim for removal after the 19.0 branch point.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2019-01-11 Thread Matt Turner
On Fri, Jan 11, 2019 at 2:17 PM Jason Ekstrand  wrote:
>
> On Fri, Jan 11, 2019 at 4:11 PM Matt Turner  wrote:
>>
>> From: Gert Wollny 
>>
>> Since Meson will eventually be the only build system deprecate autotools
>> now. It can still be used by invoking configure with the flag
>>   --enable-autotools
>>
>> NAKed-by: Ilia Mirkin 
>> Acked-by: Eric Engestrom 
>> Acked-by: Kenneth Graunke 
>> Acked-by: Lionel Landwerlin 
>> Acked-by: Jason Ekstrand 
>> Acked-by: Rob Clark 
>> Acked-by: Marek Olšák 
>> Reviewed-by: Christian Gmeiner 
>> Reviewed-by: Matt Turner 
>> Reviewed-by: Eric Anholt 
>> Signed-off-by: Gert Wollny 
>> ---
>> I think there's support for overriding the sole objection to this patch.
>>
>> To confirm:
>>
>> (1) The plan is to remove Autotools, perhaps after the 19.0 release
>>
>> (2) This patch's purpose is to ensure that everyone knows that
>> Autotools will be going away (think: people who build Mesa as
>> part of an automated process and wouldn't notice a deprecation
>> warning unless it requires some action from them)
>>
>> (3) We expect all reasonable concerns about Meson to be resolved
>> before Autotools is removed (e.g., reconfiguration problems,
>> retrieving configuration command line, configuration status
>> output, etc.)
>
>
> Do we have a tracker bug for these yet?  It'd be good to have one if we don't 
> already so that we can keep track of the reasonable blocking issues.

We did not, but that's a good idea. I've created a tracker bug:

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

and will file blocking issues for the three things I mentioned above.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2019-01-11 Thread Matt Turner
From: Gert Wollny 

Since Meson will eventually be the only build system deprecate autotools
now. It can still be used by invoking configure with the flag
  --enable-autotools

NAKed-by: Ilia Mirkin 
Acked-by: Eric Engestrom 
Acked-by: Kenneth Graunke 
Acked-by: Lionel Landwerlin 
Acked-by: Jason Ekstrand 
Acked-by: Rob Clark 
Acked-by: Marek Olšák 
Reviewed-by: Christian Gmeiner 
Reviewed-by: Matt Turner 
Reviewed-by: Eric Anholt 
Signed-off-by: Gert Wollny 
---
I think there's support for overriding the sole objection to this patch.

To confirm:

(1) The plan is to remove Autotools, perhaps after the 19.0 release

(2) This patch's purpose is to ensure that everyone knows that
Autotools will be going away (think: people who build Mesa as
part of an automated process and wouldn't notice a deprecation
warning unless it requires some action from them)

(3) We expect all reasonable concerns about Meson to be resolved
before Autotools is removed (e.g., reconfiguration problems,
retrieving configuration command line, configuration status
output, etc.)

 configure.ac | 13 +
 1 file changed, 13 insertions(+)

diff --git a/configure.ac b/configure.ac
index e4d20054d5f..c7473d77eff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -52,6 +52,19 @@ mingw*)
 ;;
 esac
 
+AC_ARG_ENABLE(autotools,
+   [AS_HELP_STRING([--enable-autotools],
+   [Enable the use of this autotools based build 
configuration])],
+   [enable_autotools=$enableval], [enable_autotools=no])
+
+if test "x$enable_autotools" != "xyes" ; then
+AC_MSG_ERROR([the autotools build system has been deprecated in favour of
+meson and will be removed eventually. For instructions on how to use meson
+see https://www.mesa3d.org/meson.html.
+If you still want to use the autotools build, then add --enable-autotools
+to the configure command line.])
+fi
+
 # Support silent build rules, requires at least automake-1.11. Disable
 # by either passing --disable-silent-rules to configure or passing V=1
 # to make
-- 
2.19.2

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


Re: [Mesa-dev] [PATCH 05/12] nir: rename global/local to private/function memory

2019-01-11 Thread Matt Turner
On Fri, Jan 11, 2019 at 9:11 AM Kenneth Graunke  wrote:
>
> On Friday, January 11, 2019 8:33:41 AM PST Jason Ekstrand wrote:
> > On Fri, Jan 11, 2019 at 10:19 AM Kenneth Graunke wrote:
> > > Those names (nir_var_func_local, nir_var_thread_local, and
> > > nir_var_thread_global) make more sense to me than private/function.
> > >
> > > Another option is `nir_var_local_temp` and `nir_var_shader_temp`,
> > > indicating that they're just temporary variables, and not anything
> > > with special semantics like memory.  shader_temp would pair well with
> > > the existing shader_in/shader_out, since they have the same scope.
> > >
> > > I might also consider adding 'mem' to variables representing memory.
> > >
> > > So that would look like...
> > >
> > >nir_var_shader_in
> > >nir_var_shader_out
> > >nir_var_shader_temp  (formerly local/function)
> > >nir_var_local_temp   (formerly global/private)
> > >
> >
> > Are those flipped?
>
> Gah!  Sorry.  Yes.
>
>nir_var_shader_in
>nir_var_shader_out
>nir_var_shader_temp  (formerly global/private)
>nir_var_local_temp   (formerly local/function)
>nir_var_uniform
>nir_var_system_value
>nir_var_mem_ubo  (added mem)
>nir_var_mem_ssbo (added mem)
>nir_var_mem_shared   (added mem)
>nir_var_mem_global   (the new global memory type being introduced)

That sounds good to me. Thanks for coming up with those names.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-21 Thread Matt Turner
On Fri, Dec 21, 2018 at 1:36 AM Stuart Young  wrote:
> Yes, but at the moment we have no consensus and a deadlock on the issue.

Consensus need not be unanimous. We have consensus.

Signed-off-by: Gert Wollny 
Reviewed-by: Matt Turner 
Acked-by: Eric Engestrom 
Acked-by: Kenneth Graunke 

and I would probably include Jason and Marek in this group as well,
though they did not offer a *-by.

I don't think there's anything to continue debating, so this will be
my last contribution to the thread.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] MR: i965: Enable software support for ARB_gpu_shader_fp64/ARB_gpu_shader_int64

2018-12-20 Thread Matt Turner
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/39

Some upcoming Intel platforms do not have 64-bit type support. This
series implements support for ARB_gpu_shader_fp64 and
ARB_gpu_shader_int64 via software implementations.

To do this we add a .glsl file containing GLSL implementations of
various 64-bit operations (e.g., double-precision addition) and
compile them to NIR when we see that the "user shader" uses some
64-bit operations. Lowering is done in nir_lower_doubles() but a
little differently than normal: we convert the to-be-lowered
operations into function calls of the software routines and ultimately
inline the code. In the future we may wish to use real function calls.

I have a few known test failures (seem to be mostly about
ARB_enhanced_layouts -- which makes since given that we're faking
64-bit types) but I think this is good enough to merge. I hope after
review you will agree!

 src/compiler/Makefile.glsl.am  |7 +
 src/compiler/glsl/ast_function.cpp |5 +-
 src/compiler/glsl/float64.glsl | 1744 
 src/compiler/glsl/glsl_to_nir.cpp  |  125 +-
 src/compiler/glsl/meson.build  |   10 +-
 src/compiler/glsl/xxd.py   |  109 ++
 src/compiler/nir/nir.c |1 +
 src/compiler/nir/nir.h |   34 +-
 src/compiler/nir/nir_algebraic.py  |7 +-
 src/compiler/nir/nir_builder.h |1 +
 src/compiler/nir/nir_clone.c   |1 +
 src/compiler/nir/nir_gather_info.c |5 +
 src/compiler/nir/nir_inline_functions.c|4 +
 src/compiler/nir/nir_lower_constant_initializers.c |   32 +-
 src/compiler/nir/nir_lower_double_ops.c|  208 ++-
 src/compiler/nir/nir_lower_global_vars_to_local.c  |8 +
 src/compiler/nir/nir_lower_int64.c |  471 +-
 src/compiler/nir/nir_lower_load_const_to_scalar.c  |7 +-
 src/compiler/nir/nir_lower_returns.c   |4 +
 src/compiler/nir/nir_lower_var_copies.c|7 +-
 src/compiler/nir/nir_lower_vars_to_ssa.c   |6 +-
 src/compiler/nir/nir_opt_constant_folding.c|7 +-
 src/compiler/nir/nir_opt_copy_prop_vars.c  |4 +
 src/compiler/nir/nir_opt_copy_propagate.c  |4 +
 src/compiler/nir/nir_opt_cse.c |7 +-
 src/compiler/nir/nir_opt_dce.c |7 +-
 src/compiler/nir/nir_opt_dead_cf.c |7 +-
 src/compiler/nir/nir_opt_if.c  |4 +
 src/compiler/nir/nir_opt_peephole_select.c |7 +-
 src/compiler/nir/nir_opt_remove_phis.c |4 +
 src/compiler/nir/nir_opt_undef.c   |7 +-
 src/compiler/nir/nir_serialize.c   |4 +
 src/compiler/nir/nir_split_var_copies.c|4 +
 src/compiler/shader_info.h |5 +
 src/compiler/spirv/spirv_to_nir.c  |1 +
 src/intel/compiler/brw_eu_emit.c   |   12 +-
 src/intel/compiler/brw_fs.cpp  |   68 +-
 src/intel/compiler/brw_fs.h|2 +-
 src/intel/compiler/brw_fs_combine_constants.cpp|7 +-
 src/intel/compiler/brw_fs_generator.cpp|3 +-
 src/intel/compiler/brw_fs_nir.cpp  |4 +-
 src/intel/compiler/brw_fs_reg_allocate.cpp |2 +-
 src/intel/compiler/brw_fs_register_coalesce.cpp|8 +-
 src/intel/compiler/brw_nir.c   |   91 +-
 src/intel/compiler/brw_reg.h   |7 +-
 src/intel/compiler/brw_schedule_instructions.cpp   |   14 +-
 src/intel/compiler/brw_vec4.cpp|   12 +-
 src/intel/compiler/brw_vec4.h  |2 +-
 src/intel/compiler/brw_vec4_reg_allocate.cpp   |   12 +-
 src/intel/compiler/brw_vec4_visitor.cpp|4 +-
 src/intel/compiler/gen6_gs_visitor.cpp |4 +-
 src/mesa/drivers/dri/i965/Makefile.am  |1 +
 src/mesa/drivers/dri/i965/brw_program.c|   61 +
 src/mesa/drivers/dri/i965/intel_extensions.c   |8 +-
 src/mesa/drivers/dri/i965/meson.build  |2 +-
 55 files changed, 3061 insertions(+), 131 deletions(-)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-19 Thread Matt Turner
On Wed, Dec 19, 2018 at 12:06 PM Ilia Mirkin  wrote:
> > We're simply trying to get the feedback from users sooner. And the
> > cost to you is very small: Use an extra flag. It's not a burden.
>
> Before the community is happy? Premature. The way you build consensus
> for a new thing is not by shooting the old thing in the foot. That
> just encourages me to pick up a shotgun too.

This reply has convinced me that you're not to be reasoned with.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-19 Thread Matt Turner
On Wed, Dec 19, 2018 at 10:32 AM Ilia Mirkin  wrote:
>
> On Wed, Dec 19, 2018 at 10:25 AM Matt Turner  wrote:
> >
> > On Wed, Dec 19, 2018 at 8:06 AM Ilia Mirkin  wrote:
> > >
> > > On Wed, Dec 19, 2018 at 1:01 AM Matt Turner  wrote:
> > > > WTF would you have us do?
> > >
> > > Same thing as for any change with an impact this wide --
> > >
> > > 1. Identify stakeholders. In this case, probably the sub-project
> > > maintainers, major contributors, and a smattering of distro
> > > maintainers.
> > > 2. Make them happy, or at least get them, as a group, to agree that
> > > it's "good enough".
> >
> > So we're trying to get better coverage than what you're suggesting.
> >
> > > 3. Apply.
> > >
> > > This is the point at which you can make autotools less visible. We're
> > > not at that point yet.
> >
> > Ilia, it's an extra flag. I think you'll survive.
>
> It's an advertising strategy for meson (hello world, check this out,
> it's going to be the default soon). It can be done at the final stage.
> We're not at that stage.

It's an attempt to get testing from users that might not know about it
yet. It's not as simple as advertising. If you're confused about the
intention of the patch, consider that it's from Gert who has not been
enthusiastic about removing autotools.

> We're at the stage of "hello community, we'd like to replace
> autotools", and the community coming back to you with feedback.

People have been using Meson quite successfully for more than a year
now. It's now used in Gentoo's mesa ebuild for six months and is used
in the stable version. Your claim is subjective, but I think it's
pretty clear that it's not accurate.

Regardless of all of that, what you're suggesting is only marking
autotools as deprecated (i.e., this patch) after all known problems
with the Meson build are fixed. At that point in 3 months down the
line if we get another bug report from a user that only learned of
Meson from this patch, you're going to advocate for keeping autotools
for another release. Rinse, repeat.

We're simply trying to get the feedback from users sooner. And the
cost to you is very small: Use an extra flag. It's not a burden.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-19 Thread Matt Turner
On Wed, Dec 19, 2018 at 8:06 AM Ilia Mirkin  wrote:
>
> On Wed, Dec 19, 2018 at 1:01 AM Matt Turner  wrote:
> > WTF would you have us do?
>
> Same thing as for any change with an impact this wide --
>
> 1. Identify stakeholders. In this case, probably the sub-project
> maintainers, major contributors, and a smattering of distro
> maintainers.
> 2. Make them happy, or at least get them, as a group, to agree that
> it's "good enough".

So we're trying to get better coverage than what you're suggesting.

> 3. Apply.
>
> This is the point at which you can make autotools less visible. We're
> not at that point yet.

Ilia, it's an extra flag. I think you'll survive.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-18 Thread Matt Turner
On Tue, Dec 18, 2018 at 6:53 PM Ilia Mirkin  wrote:
>
> On Tue, Dec 18, 2018 at 6:40 PM Gert Wollny  wrote:
> >
> > Hi Ilia,
> >
> > Am Sonntag, den 16.12.2018, 12:40 -0500 schrieb Ilia Mirkin:
> > > On Sun, Dec 16, 2018 at 6:24 AM Gert Wollny 
> > > wrote:
> > > >
> > > > Since Meson will eventually be the only build system deprecate
> > > > autotools
> > > > now. It can still be used by invoking configure with the flag
> > > >   --enable-autotools
> > > >
> >
> > [...]
> > > If the concern is that there are 2 build systems and it's unclear
> > > which to use, I'd definitely err on the side of making meson the one
> > > requiring extra hoops to jump through.
> > >
> >
> > I think Jason already pointed out that this patch doesn't remove
> > anything. If you have followed the threads you've seen that I'm also
> > not exactly pushing for removing autotools, but I think since it is
> > going to happen anyway, it would be best to give everyone a little more
> > incentive to try out meson so that hopefully all the remaining issue
> > are brought up and can be fixed, before autotools is removed in a
> > stable release.
>
> Yeah, I totally see that you're not removing autotools with this
> patch. I do see that you're making it harder to use though (i.e.
> indicating to users that it's not the #1 way to do things). This
> should only be done when it's ready to be deprecated. The argument
> I've been making is that it's not. I've outlined some things that I
> think are important to fix in meson in another thread.

You've said Meson's not ready given X, Y, and Z use cases. I've noted
that some of those use cases were not reported earlier because the
users had not given Meson a try until we began discussing removing
autotools. This patch attempts to simply inform a larger group of
users that are not subscribed to mesa-dev@ that the intention is for
autotools to be removed.

Dylan is working to fix X, Y, and Z, but there's a reasonable chance
that a user who wasn't subscribed to mesa-dev@ might find problems A,
B, and C only after he's forced to switch to Meson. This patch is the
only thing that's been proposed so far to communicate with those
users. Without some required interaction on the users part we will
fail to communicate with some users and we'll be right back where we
are now.

WTF would you have us do?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 12/28] util: add fp64 -> fp32 conversion support for RTNE and RTZ rounding modes

2018-12-17 Thread Matt Turner
On Mon, Dec 10, 2018 at 11:25 AM Samuel Iglesias Gonsálvez
 wrote:
>
> On 07/12/2018 03:03, Matt Turner wrote:
> > On Wed, Dec 5, 2018 at 7:56 AM Samuel Iglesias Gonsálvez
> >  wrote:
> >>
> >> Signed-off-by: Samuel Iglesias Gonsálvez 
> >> ---
> >>  src/util/Makefile.sources |   2 +
> >>  src/util/double.c | 197 ++
> >>  src/util/double.h |  46 +
> >>  src/util/meson.build  |   2 +
> >>  4 files changed, 247 insertions(+)
> >>  create mode 100644 src/util/double.c
> >>  create mode 100644 src/util/double.h
> >
> > Why do we need software routines for this? Couldn't we set/reset the
> > rounding mode (fegetround/fegetround) around a double -> float
> > conversion?
> >
>
> Yes, that was my first idea. However, I found an issue with GCC that
> forces me to do these software routines instead.
>
> I implemented _mesa_double_to_float_rtz() as:
>
> float
> _mesa_double_to_float_rtz(double val)
> {
>int curr_method = fegetround();
>float result;
>fesetround(FE_TOWARDZERO);
>result = val;
>fesetround(curr_method);
>return result;
> }
>
> If I add a printf, I got the proper value. However if I remove it, the
> result value is using the default's rounding mode (FE_TONEAREST). I
> think it is reordering instructions or optimizing something. I have set
> #pragma STDC FENV_ACCESS ON but it doesn't work either.
>
> Updated news: I give it another spin. I defined result as volatile
> variable and that seems to work :) My GCC knowledge is limited, so if
> someone brings a better idea to fix this, I will be delighted to test
> it. If not, I will send a v2 version with the volatile and
> fesetround()/fegetround() functions.

Dang, I had no idea it wasn't really possible to safely set the
rounding mode for conversions like this. I see lots of articles saying
"#pragma STDC FENV_ACCESS ON" isn't supported. I think your original
code might be the best thing to do.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] WIP: meson: allow building DRI loaders without a DRI driver

2018-12-17 Thread Matt Turner
On Mon, Dec 17, 2018 at 2:12 PM Emil Velikov  wrote:
> Additionally, distributions build latest loader and use it with DRI1
> era drivers.

I'm curious if there are distributions that still ships the DRI1
drivers. Do you know which, if any, do?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools

2018-12-16 Thread Matt Turner
On Sun, Dec 16, 2018 at 6:24 AM Gert Wollny  wrote:
>
> Since Meson will eventually be the only build system deprecate autotools
> now. It can still be used by invoking configure with the flag
>   --enable-autotools
>
> Signed-off-by: Gert Wollny 
> ---
> IMO autotools should be properly deprecated prior it its removal, so here
> is a patch to do just that. I think autotools should be marked as deprecated
> for the 19.0 release and, depending on feedback, it could be removed with 
> 19.1.
> Anyway, in the end it's up to the release team how to handle this.
>
> Best,
> Gert
>
>  configure.ac | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 9b437a252c..73f5978bb7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -52,6 +52,19 @@ mingw*)
>  ;;
>  esac
>
> +AC_ARG_ENABLE(autotools,
> +   [AS_HELP_STRING([--enable-autotools],
> +   [Enable the use of this autotools based build 
> configuration])],
> +   [enable_autotools=$enableval], [enable_autotools=no])
> +
> +if test "x$enable_autotools" != "xyes" ; then
> +AC_MSG_ERROR([the autotools build system has been deprecated in favour of
> +meson and will be removed eventually. For instructions on how to use 
> meson
> +see https://www.mesa3d.org/meson.html.
> +If you still want to use the autotools build, then add --enable-autotools
> +to the configure command line.])
> +fi
> +

Fine by me.

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


Re: [Mesa-dev] last call for autotools

2018-12-15 Thread Matt Turner
On Fri, Dec 14, 2018 at 11:40 AM Ilia Mirkin  wrote:
>
> On Fri, Dec 14, 2018 at 11:32 AM Matt Turner  wrote:
> >
> > On Fri, Dec 14, 2018 at 4:12 AM Gert Wollny  wrote:
> > > I second that, I voiced my concerns in a former thread, especially that
> > > so far this upcoming change has not been officially announced in the
> > > release notes or on mesa-user, and that I don't understand why it is so
> > > urgent to drop autotools when there is still someone who offers to
> > > maintain it and some who prefer to use it.
> >
> > It's because the objective is to have as few build systems as
> > necessary. This is an obvious first step and has been a stated goal
> > for more than a year. I'm surprised that you're surprised.
> >
> > Let's just make an external contrib repo and keep all the autotools
> > files there. Then Emil and company can continue having fun maintaining
> > autotools and all you'll need to do is rsync into the Mesa repo to use
> > autotools.
>
> So ... not last call for autotools? More like "railroad notification
> that the train is in motion"?

I wouldn't say that, but I think it is pretty apparent that people
haven't attempted to switch before some of these recent discussions.
So.. not sure how better to handle it.

> This kind of talk just causes everyone to entrench into their
> positions, so I'd rather avoid it. I'm not opposed to switching build
> systems in principle, but the current proposal is, in my and seemingly
> some other people's views, inadequate. Let's make it adequate before
> dropping the thing that works for everyone.

How can we make it adequate if people never even test the thing we've
been talking about deleting autotools in favor or for 15 months?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] last call for autotools

2018-12-14 Thread Matt Turner
On Fri, Dec 14, 2018 at 4:12 AM Gert Wollny  wrote:
> I second that, I voiced my concerns in a former thread, especially that
> so far this upcoming change has not been officially announced in the
> release notes or on mesa-user, and that I don't understand why it is so
> urgent to drop autotools when there is still someone who offers to
> maintain it and some who prefer to use it.

It's because the objective is to have as few build systems as
necessary. This is an obvious first step and has been a stated goal
for more than a year. I'm surprised that you're surprised.

Let's just make an external contrib repo and keep all the autotools
files there. Then Emil and company can continue having fun maintaining
autotools and all you'll need to do is rsync into the Mesa repo to use
autotools.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] last call for autotools

2018-12-13 Thread Matt Turner
On Thu, Dec 13, 2018 at 10:19 PM Ilia Mirkin  wrote:
> So now what? I don't remember how that config was done, except that it
> was done the way I decided I needed it at the time. I have no way to
> recover it. With autotools, in such cases (which are immensely rare),
> you just run "head config.log" and it tells you what you did last
> time. And by updating the build component, now I have to rebuild
> EVERYTHING?

Since it uses ccache, that should take about 20 seconds...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] nir: Document the function inlining process

2018-12-11 Thread Matt Turner
Thanks. This was very useful for me in the fp64 work.

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


Re: [Mesa-dev] [PATCH] intel/compiler: Set swizzle to BRW_SWIZZLE_XXXX for scalar region

2018-12-10 Thread Matt Turner
On Sat, Dec 8, 2018 at 9:51 PM Sagar Ghuge  wrote:
>
> When RepCtrl is set, the swizzle field is ignored by the hardware. In
> order to ensure a 1-to-1 correspondence between the human-readable
> disassembly and the binary instruction encoding always set the swizzle
> to  (all zeros) when it is unused due to RepCtrl
>
> Signed-off-by: Sagar Ghuge 
> Reviewed-by: Matt Turner 

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


Re: [Mesa-dev] [PATCH v2] intel/compiler: Always print flag subregister number

2018-12-10 Thread Matt Turner
On Sat, Dec 8, 2018 at 11:08 PM Sagar Ghuge  wrote:
>
> While disassembling the predicate always print flag subregister number
> to keep grammar same across the generation for assembler tool.
>
> v2: Club consecutive format calls (Matt Turner)
>
> Signed-off-by: Sagar Ghuge 

Reviewed-by: Matt Turner 

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


Re: [Mesa-dev] [PATCH 11/28] util: added float to float16 conversions with RTZ and RTNE

2018-12-10 Thread Matt Turner
On Mon, Dec 10, 2018 at 6:21 AM Samuel Iglesias Gonsálvez
 wrote:
>
> Hello Matt,
>
> On 07/12/2018 03:20, Matt Turner wrote:
> > Since this is for an extension that will be BDW+ can we use the
> > _cvtss_sh() intrinsic instead? It corresponds to an IVB+ instruction
> > and even takes the rounding mode directly as an immediate argument.
> >
>
> If I understand currently this thread, then you withdraw this suggestion
> because of the reasons given by Roland, right?

Yes, that's right.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] meson: Bump version to 0.46 for python module

2018-12-08 Thread Matt Turner
On Thu, Nov 22, 2018 at 1:05 PM Matt Turner  wrote:
>
> On Wed, Nov 21, 2018 at 10:48 AM Matt Turner  wrote:
> >
> > Thanks Arfrever and Dylan.
> >
> > Acked-by: Matt Turner 
>
> Hmm, actually this doesn't seem to work for me. With it applied I get:
>
> src/mesa/drivers/dri/meson.build:59:8: ERROR:  Python object does not
> have method path.
>
> which comes from:
>
>   meson.add_install_script(
> prog_python.path(),
> join_paths(meson.source_root(), 'bin/install_megadrivers.py'),
> libmesa_dri_drivers.full_path(),
> dri_drivers_path,
> dri_link,
>   )

Ping. I think this one fell through the cracks.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 11/28] util: added float to float16 conversions with RTZ and RTNE

2018-12-06 Thread Matt Turner
On Thu, Dec 6, 2018 at 7:22 PM Roland Scheidegger  wrote:
>
> Am 07.12.18 um 03:20 schrieb Matt Turner:
> > Since this is for an extension that will be BDW+ can we use the
> > _cvtss_sh() intrinsic instead? It corresponds to an IVB+ instruction
> > and even takes the rounding mode directly as an immediate argument.
>
> Not saying trying to use it isn't a good idea, but you'd need the right
> compile flags, and you can't assume it's present, since even the latest
> pentiums don't support avx (and by extension, f16c). (The same is true
> for atoms too, of course).

I'm not sure that AVX and F16C are related, but from a quick glance it
seems that you're right that Atoms ("little core") doesn't support
F16C. I had no idea :(

As far as I can tell all "big cores" have F16C. That's what
https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html indicates.

If we've got to have the code, we might as well use it and not
complicate it by using _cvtss_sh() then. Dang.

(Unfortunately there seems to be bad information out there confusing
things though... see https://communities.intel.com/thread/121635)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 11/28] util: added float to float16 conversions with RTZ and RTNE

2018-12-06 Thread Matt Turner
Since this is for an extension that will be BDW+ can we use the
_cvtss_sh() intrinsic instead? It corresponds to an IVB+ instruction
and even takes the rounding mode directly as an immediate argument.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 12/28] util: add fp64 -> fp32 conversion support for RTNE and RTZ rounding modes

2018-12-06 Thread Matt Turner
On Wed, Dec 5, 2018 at 7:56 AM Samuel Iglesias Gonsálvez
 wrote:
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>  src/util/Makefile.sources |   2 +
>  src/util/double.c | 197 ++
>  src/util/double.h |  46 +
>  src/util/meson.build  |   2 +
>  4 files changed, 247 insertions(+)
>  create mode 100644 src/util/double.c
>  create mode 100644 src/util/double.h

Why do we need software routines for this? Couldn't we set/reset the
rounding mode (fegetround/fegetround) around a double -> float
conversion?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Make Jordan an Owner of the mesa project?

2018-12-05 Thread Matt Turner
On Tue, Dec 4, 2018 at 7:39 PM Jason Ekstrand  wrote:
>
> It's been 24 hours and the only owner who hasn't replied yet is Matt.  Given 
> that everyone else has firmly ACKed, I'm going to click the button.  
> Congratulations, Jordan, you're now a mesa Owner!

That's certainly no reflection on my opinion of Jordan :)

(I've just been sick)

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


Re: [Mesa-dev] [PATCH mesa] drop autotools

2018-12-04 Thread Matt Turner
On Tue, Dec 4, 2018 at 3:43 PM Gert Wollny  wrote:
>
> Am Dienstag, den 04.12.2018, 14:01 -0800 schrieb Matt Turner:
> > On Tue, Dec 4, 2018 at 12:48 PM Gert Wollny 
> > wrote:
> > > FWIW, I also don't understand the urge to remove the automake build
> > > system files, they account for less then 1% of line count in the
> > > source
> > > tree, Emil offered to maintain the build system in a way that
> > > nobody
> > > who doesn't want to deal with has to touch it
> >
> > Do you see any value in removing the autotools build system at all?
>
> If there was nobody willing to maintain it and nobody who wants to use
> it then I would say it makes sense to remove it, because it doesn't
> make sense to keep something that is unused and will bitrot, but right
> now this is not the case. In light of what Emil wrote I could also ask:
> what's the harm in keeping it?

Noted, but I think your opinion is out of step even with those others
asking to keep autotools around a while longer.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH mesa] drop autotools

2018-12-04 Thread Matt Turner
On Tue, Dec 4, 2018 at 12:48 PM Gert Wollny  wrote:
> FWIW, I also don't understand the urge to remove the automake build
> system files, they account for less then 1% of line count in the source
> tree, Emil offered to maintain the build system in a way that nobody
> who doesn't want to deal with has to touch it

Do you see any value in removing the autotools build system at all?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-04 Thread Matt Turner
On Tue, Dec 4, 2018 at 3:15 AM Juan A. Suarez Romero
 wrote:
>
> On Mon, 2018-12-03 at 15:38 -0800, Matt Turner wrote:
> > On Mon, Dec 3, 2018 at 8:12 AM Emil Velikov  
> > wrote:
> > > On Thu, 29 Nov 2018 at 23:54, 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.
> > > >
> > > Kind of unfortunate but I can see where you're coming from.
> > > No tests, thus one cannot verify the extension works correctly and
> > > ensures it stays OK.
> > >
> > > Fwiw I couldn't spot any dEQP tests either :-(
> > >
> > > > 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)
> > > >
> > > Are you planning to push this to master? Or you'd like it reverted
> > > only for the 18.3 branch?
> >
> > Just pushed to master. Please include in 18.3.
>
>
> This was committed without explicitly CCing to 18.3 stable release, so it is
> shown as a candidate to 18.2 stable release too.

Hm, I thought that the scripts read the "This reverts commit $SHA1"
and did the right thing. Or was it my Cc: mesa-stable that caused the
confusion?

> As I think this is not what we want, I'll ignore this commit for 18.2.

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


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

2018-12-03 Thread Matt Turner
On Mon, Dec 3, 2018 at 8:12 AM Emil Velikov  wrote:
>
> On Thu, 29 Nov 2018 at 23:54, 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.
> >
> Kind of unfortunate but I can see where you're coming from.
> No tests, thus one cannot verify the extension works correctly and
> ensures it stays OK.
>
> Fwiw I couldn't spot any dEQP tests either :-(
>
> > 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)
> >
> Are you planning to push this to master? Or you'd like it reverted
> only for the 18.3 branch?

Just pushed to master. Please include in 18.3.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 00/29] do not allow invalid texture-format enums

2018-11-30 Thread Matt Turner
On Fri, Nov 30, 2018 at 4:46 AM Francesco Ansanelli  wrote:
> Il giorno ven 30 nov 2018, 12:26 Erik Faye-Lund 
>  ha scritto:
>>
>> On Fri, 2018-11-23 at 11:53 +0100, Erik Faye-Lund wrote:
>> > OK, so here's a v2 of this series. These are the changes since v1:
>> > - Removed double-semicolons in patch #25
>> > - Removed default-case and questionable comment in patch #25
>> > - Dropped patch #30, as it would regress functionality on two drivers
>> >
>> > Please review :)
>>
>> Ping? Any takers?
>
> I read all the patches and didn't spot anything wrong, but please don't 
> consider this a proper review.

Thank you for speaking up. Even if you don't feel like your
Reviewed-by tag is sufficient it's still valuable! Feel free to give
it when you have reviewed patches to the best of your ability.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Lets talk about autotools

2018-11-30 Thread Matt Turner
On Fri, Nov 30, 2018 at 2:28 AM Gert Wollny  wrote:
> Am Donnerstag, den 29.11.2018, 17:44 + schrieb 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.
>
> I second that, I think the process of removing autotools should be a
> two-step procedure. i.e. prior to ripping autotools support out there
> should be one or two releases that deprecates it, e.g. by changing
> configure that it (1) needs an extra flag to run, and (2) when run
> without this flag it would just print a message about the meson build
> system, the deprecation of autotools (making it clear when it will be
> removed), and information how to still run autotools with this extra
> flag. If qwe
>
> A rationale is that with a release that only has mesa there is a high
> chance that people not directly involved with the project, and that
> don't follow git but only use the releases hit corner cases when
> building mesa that we or maintainers for distributions might not be
> aware of. Still having something around that is known to work would be
> good for them, so they can report a bug against the meson build system
> and still get their work done by easily switching to the autotools for
> the time being.

I've been using Meson to build Mesa in Gentoo for a ~6 months now, and
I've reported (and Dylan has fixed) a handful of corner case bugs.
Dylan is super responsive, usually having a fix within a day. It's
been great, really.

And we've been discussing removing autotools for more than a year now.

I think we've been plenty conservative and there's nothing to be
gained / no problems to be avoided by delaying further. (And again, if
someone comes out of the woodwork with a bug in the Meson build, Dylan
will fix it within a day in my experience)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Lets talk about autotools

2018-11-29 Thread 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


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

2018-11-29 Thread 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] Lets talk about autotools

2018-11-29 Thread 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

2018-11-29 Thread 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] egl: define NULL in egldevice.h

2018-11-27 Thread Matt Turner
Reviewed-by: Matt Turner 

I'll commit it tomorrow.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] gallivm: Use nextafterf(0.5, 0.0) as rounding constant

2018-11-27 Thread Matt Turner
The common truncf(x + 0.5) fails for the floating-point value just less
than 0.5 (nextafterf(0.5, 0.0)). nextafterf(0.5, 0.0) + 0.5, after
rounding is 1.0, thus truncf does not produce the desired value.

The solution is to add nextafterf(0.5, 0.0) instead of 0.5 before
truncating. This works for all values.
---
I noticed this while investigating https://bugs.gentoo.org/665570 but it
does not fix it.

Roland, do you have a suggestion for how to make lp_build_iround() work
on non-SSE/non-Altivec platforms? I notice that if I unconditionally
return TRUE from arch_rounding_available() and make
lp_build_round_arch() take the SSE4.1 path (that emits llvm.nearbyint)
it passes on ARM64.

I noticed there's some hack in lp_test_arit.c:test_unary:

   if (test->ref ==  && length == 2 &&
   ref != roundf(testval)) {
  /* FIXME: The generic (non SSE) path in lp_build_iround, which is
   * always taken for length==2 regardless of native round support,
   * does not round to even. */
  expected_pass = FALSE;
   }

It'd be nice to get rid of that.. but maybe we can somehow use it to
just mark all the round tests as expected fail on other platforms if no
real fix is forthcoming?

 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index f348833206b..c050bfdb936 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -2477,7 +2477,7 @@ lp_build_iround(struct lp_build_context *bld,
else {
   LLVMValueRef half;
 
-  half = lp_build_const_vec(bld->gallivm, type, 0.5);
+  half = lp_build_const_vec(bld->gallivm, type, nextafterf(0.5, 0.0));
 
   if (type.sign) {
  LLVMTypeRef vec_type = bld->vec_type;
-- 
2.18.1

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


  1   2   3   4   5   6   7   8   9   10   >