Re: Intel clc dependency

[Brian Paul]

On 4/10/24 13:53, Timo Aaltonen wrote:

Brian Paul kirjoitti 6.4.2024 klo 1.05:
I'm trying to build the Intel Vulkan driver.  First time in a few 
months.  I'm having build problems related to clc.  I'm on Ubuntu 22.04



[...]
[1347/3181] Generating src/intel/vulkan/...om command (wrapped by 
meson to set env)
FAILED: 
src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h
env MESA_SHADER_CACHE_DISABLE=true MESA_SPIRV_LOG_LEVEL=error 
/home/brianp/build3/mesa/build/src/intel/compiler/intel_clc -p dg2 
--prefix gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable -e 
BFS_pass1_indexed_batchable --in 
../src/intel/vulkan/grl/gpu/bvh_build_BFS.cl --in 
/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu/libs/lsc_intrinsics_fallback.cl -o src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h -- -cl-std=cl2.0 -D__OPENCL_VERSION__=200 -DMAX_HW_SIMD_WIDTH=16 -DMAX_WORKGROUP_SIZE=16 -I/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu -I/home/brianp/build3/mesa/src/intel/vulkan/grl/include

ERROR: libclc shader missing. Consider installing the libclc package
Aborted (core dumped)

I've installed every clc-related package I could find.  I've tried 
several options for the 'intel-clc' option without luck.


BTW, the description of intel-clc in meson_options.txt looks suspect:

option(
   'intel-clc',
   type : 'combo',
   deprecated: {'true': 'enabled', 'false': 'disabled'},
   value : 'disabled',
   choices : [
 'enabled', 'disabled', 'system',
   ],
   description : 'Build the intel-clc compiler (enables Vulkan Intel ' +
 'Ray Tracing on supported hardware).'
)

The default is 'disabled' but that's deprecated?  Choices include 
'enabled' but that's deprecated too?


Any tips for building the ToT Intel Vulkan driver?

-Brian



You need to have libclc-NN-dev installed matching with the llvm version, 
which on stock 22.04 would be libclc-13-dev.


I'm using llvm 15 and have libclc-15-dev installed.  I get the "ERROR: 
libclc shader missing. Consider installing the libclc package" issue I 
quoted above.


I have llvm 15 installed because I also want to build the radv Vulkan 
driver.


-Brian


--
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Re: Intel clc dependency

[Brian Paul]

Thanks, Mike.  That works as I don't need RT functionality.

BTW, I was looking at a stale meson_options.txt file so ignore my 
comments about that.


-Brian


On 4/5/24 16:24, Mike Blumenkrantz wrote:
This doesn't solve the problem about missing CLC, but I pass 
-Dintel_rt=disabled to avoid the whole thing.


On Fri, Apr 5, 2024, 6:05 PM Brian Paul <mailto:brian.p...@broadcom.com>> wrote:


I'm trying to build the Intel Vulkan driver.  First time in a few
months.  I'm having build problems related to clc.  I'm on Ubuntu 22.04


[...]
[1347/3181] Generating src/intel/vulkan/...om command (wrapped by meson
to set env)
FAILED:
src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h
env MESA_SHADER_CACHE_DISABLE=true MESA_SPIRV_LOG_LEVEL=error
/home/brianp/build3/mesa/build/src/intel/compiler/intel_clc -p dg2
--prefix gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable -e
BFS_pass1_indexed_batchable --in
../src/intel/vulkan/grl/gpu/bvh_build_BFS.cl --in
/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu/libs/lsc_intrinsics_fallback.cl 
<http://lsc_intrinsics_fallback.cl>
-o
src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h
-- -cl-std=cl2.0 -D__OPENCL_VERSION__=200 -DMAX_HW_SIMD_WIDTH=16
-DMAX_WORKGROUP_SIZE=16
-I/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu
-I/home/brianp/build3/mesa/src/intel/vulkan/grl/include
ERROR: libclc shader missing. Consider installing the libclc package
Aborted (core dumped)

I've installed every clc-related package I could find.  I've tried
several options for the 'intel-clc' option without luck.

BTW, the description of intel-clc in meson_options.txt looks suspect:

option(
    'intel-clc',
    type : 'combo',
    deprecated: {'true': 'enabled', 'false': 'disabled'},
    value : 'disabled',
    choices : [
      'enabled', 'disabled', 'system',
    ],
    description : 'Build the intel-clc compiler (enables Vulkan
Intel ' +
                  'Ray Tracing on supported hardware).'
)

The default is 'disabled' but that's deprecated?  Choices include
'enabled' but that's deprecated too?

Any tips for building the ToT Intel Vulkan driver?

-Brian

-- 
This electronic communication and the information and any files

transmitted
with it, or attached to it, are confidential and are intended solely
for
the use of the individual or entity to whom it is addressed and may
contain
information that is confidential, legally privileged, protected by
privacy
laws, or otherwise restricted from disclosure to anyone else. If you
are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or
copying of
this e-mail is strictly prohibited. If you received this e-mail in
error,
please return the e-mail to the sender, delete it from your
computer, and
destroy any printed copy of it.




--
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Intel clc dependency

[Brian Paul]

I'm trying to build the Intel Vulkan driver.  First time in a few 
months.  I'm having build problems related to clc.  I'm on Ubuntu 22.04



[...]
[1347/3181] Generating src/intel/vulkan/...om command (wrapped by meson 
to set env)
FAILED: 
src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h
env MESA_SHADER_CACHE_DISABLE=true MESA_SPIRV_LOG_LEVEL=error 
/home/brianp/build3/mesa/build/src/intel/compiler/intel_clc -p dg2 
--prefix gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable -e 
BFS_pass1_indexed_batchable --in 
../src/intel/vulkan/grl/gpu/bvh_build_BFS.cl --in 
/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu/libs/lsc_intrinsics_fallback.cl 
-o 
src/intel/vulkan/grl/gfx125_bvh_build_BFS_BFS_pass1_indexed_batchable.h 
-- -cl-std=cl2.0 -D__OPENCL_VERSION__=200 -DMAX_HW_SIMD_WIDTH=16 
-DMAX_WORKGROUP_SIZE=16 
-I/home/brianp/build3/mesa/src/intel/vulkan/grl/gpu 
-I/home/brianp/build3/mesa/src/intel/vulkan/grl/include

ERROR: libclc shader missing. Consider installing the libclc package
Aborted (core dumped)

I've installed every clc-related package I could find.  I've tried 
several options for the 'intel-clc' option without luck.


BTW, the description of intel-clc in meson_options.txt looks suspect:

option(
  'intel-clc',
  type : 'combo',
  deprecated: {'true': 'enabled', 'false': 'disabled'},
  value : 'disabled',
  choices : [
'enabled', 'disabled', 'system',
  ],
  description : 'Build the intel-clc compiler (enables Vulkan Intel ' +
'Ray Tracing on supported hardware).'
)

The default is 'disabled' but that's deprecated?  Choices include 
'enabled' but that's deprecated too?


Any tips for building the ToT Intel Vulkan driver?

-Brian

--
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Re: Porting / AMIGA / PiStorm32 / developer needed - Back to the roots

[Brian Paul]

On 4/12/23 02:30, Ignacio Soriano Hernandez wrote:

!! External Email

Dear Mesa dev community,

Please let me apologize if you consider this off topic but I think that this is 
probably the audience that could help in reviving and getting back to the roots 
where Mesa3D originated from.

To use Brian Paul’s words:

The core library was originally (started in 1993) written on an Amiga using the 
DCC compiler. Later, development was moved to an SGI workstation.  Current 
development is done on PC/Linux systems.


Hard to believe it's been 30 years!



So thirty years later the AMIGA community is still huge, growing and the development 
on hardware and software accelerators is giving us new capabilities. One of the most 
interesting projects has been the PiStorm32 and Emu68 which enable the AMIGA to use a 
Pi as CPU accelerator with a baremetal ARM <-> 68k CPU emulator. Incredible CPU 
power but no access to the GPU and acceleration that could also boost graphics 
capabilities and applications via an RTG (Retargetable Graphics) access. The AMIGA OS 
is actively being developed (yes, still 40 years later) and AMIGA cross-compiler 
toolchains do exist for all kind of nowadays operating systems.

So what are we looking for? An experienced Mesa3D developer with and 
affinity/background for/on AMIGA that would be interested in porting an actual 
Mesa3D build to the PiStorm32/Emu68/AMIGA OS combo. Yes, 30 years later where 
it originated from. We have been collecting some funds as we know that this is 
not a weekend job but maybe someone is looking to for a challenge where „this 
is impossible“ gains your interest.


I'm not familiar with PiStorm32/Emu68 but the first big issue may be 
that Mesa makes pervasive use of 64-bit datatypes (int64_t, uint64_t). 
Just from its name, PiStorm32/Emu68 sounds like a 32-bit environment. 
So unless your compiler has good support for emulating 64-bit types with 
32-bit operations, you may be out of luck.


-Brian

Re: [Mesa-dev] Mesa (master): lavapipe: bump maxMemoryAllocationCount

[Brian Paul]

On 3/17/21 6:12 PM, GitLab Mirror wrote:

Module: Mesa
Branch: master
Commit: 23100f3b6531d7055ae4d42e07bda09d991ea438
URL:
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3D23100f3b6531d7055ae4d42e07bda09d991ea438data=04%7C01%7Cbrianp%40vmware.com%7Cd05b2a74f4bd48bf385208d8e9a28db1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C637516231677082505%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=d3Ypjgh4TYqFsQDUbFCY956sLSv1hpqG4NP2OwP4it4%3Dreserved=0

Author: Dave Airlie 
Date:   Wed Mar 17 13:33:14 2021 +1000

lavapipe: bump maxMemoryAllocationCount

not sure why this was so low


4096 is the minimum maximum that Vulkan supports.  I believe that's what 
Nvidia and AMD's Windows Vulkan drivers say.


If you want your Vulkan app to be really cross-platform it's a limit to 
be aware of.


-Brian




Reviewed-By: Mike Blumenkrantz 
Reviewed-by: Adam Jackson 
Part-of: 


---

  src/gallium/frontends/lavapipe/lvp_device.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/frontends/lavapipe/lvp_device.c 
b/src/gallium/frontends/lavapipe/lvp_device.c
index 61edf15dd0a..b087e2a7d68 100644
--- a/src/gallium/frontends/lavapipe/lvp_device.c
+++ b/src/gallium/frontends/lavapipe/lvp_device.c
@@ -574,7 +574,7 @@ VKAPI_ATTR void VKAPI_CALL 
lvp_GetPhysicalDeviceProperties(VkPhysicalDevice phys
.maxUniformBufferRange= 
pdevice->pscreen->get_shader_param(pdevice->pscreen, PIPE_SHADER_FRAGMENT, 
PIPE_SHADER_CAP_MAX_CONST_BUFFER_SIZE),
.maxStorageBufferRange= 
pdevice->pscreen->get_param(pdevice->pscreen, PIPE_CAP_MAX_SHADER_BUFFER_SIZE),
.maxPushConstantsSize = MAX_PUSH_CONSTANTS_SIZE,
-  .maxMemoryAllocationCount = 4096,
+  .maxMemoryAllocationCount = UINT32_MAX,
.maxSamplerAllocationCount= 32 * 1024,
.bufferImageGranularity   = 64, /* A cache line */
.sparseAddressSpaceSize   = 0,

___
mesa-commit mailing list
mesa-com...@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-commitdata=04%7C01%7Cbrianp%40vmware.com%7Cd05b2a74f4bd48bf385208d8e9a28db1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C637516231677082505%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=j0T9KnefIkBponHxMLxgKi8xBIMHYAydsEWV9ESBsSQ%3Dreserved=0



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

[Mesa-dev] Fwd: [Mesa-users] Surfaceless mesa 20.3.X?

[Brian Paul]

Forwarding to mesa-dev where you may get more help.  There was talk of 
changing software rendering but I'm not sure what's changed.  What 
version of Mesa are you using?


-Brian


 Forwarded Message 
Subject:[Mesa-users] Surfaceless mesa 20.3.X?
Date:   Wed, 27 Jan 2021 09:59:03 +0100
From:   Sophonet 
To: mesa-us...@lists.freedesktop.org

Hi list,

I would like to compile mesa for a surfaceless system (server, software 
rendering, no GPU). This was possible earlier with the „surfaceless“ 
platform.


The recent versions do not allow surfaceless as a platform option 
anymore. The build info which is listed for building osmesa 
(https://docs.mesa3d.org/osmesa.html) does not list a platform, and in 
that case, wayland is required, which is not available on our server.


I could build osMesa with the x11 platform, but rendering does not work 
if there is no (remote) X connection (DISPLAY missing etc.).


What is the recommended approach now?

Thanks,

sophonet


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

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

[Mesa-dev] Fwd: [Mesa-users] Issues with removal of classic OSMesa

[Brian Paul]

Hi Andreas,

I'm forwarding your message to the mesa-dev list for better visibility.

BTW, when you say "antialiasing" below, what exactly do you mean?

-Brian


 Forwarded Message 
Subject:[Mesa-users] Issues with removal of classic OSMesa
Date:   Thu, 31 Dec 2020 12:56:04 +0100
From:   Andreas Fänger 
To: mesa-us...@lists.freedesktop.org

Hi,

I've just seen that classic OSMesa has been removed (again) from Mesa3D 
a few weeks ago with this commit "mesa: Retire classic OSMesa".


We are still actively using classical OSMesa for high quality rendering 
of still images in a headless environment with no GPU support 
(server-based rendering on windows and linux)


Unfortunately, none of the alternative software renderers provide all 
the features that we require, which is antialiasing and anisotropic 
filtering. The current state is (correct me if I'm wrong)


* softpipe: anisotropic filtering is supported, no antialiasing

* llvmpipe: no anisotropic filtering, has MSAA

* openswr: no anisotropic filtering, has MSAA, no OSMesa interface (?)

We had hoped that classical OSMesa is only removed when there is a full 
replacement after the discussions in 2016 when OSMesa was about to be 
removed for the first time


https://lists.freedesktop.org/archives/mesa-dev/2016-March/109665.html

https://lists.freedesktop.org/archives/mesa-users/2016-March/001132.html

and the commit that reverted the removal

http://cgit.freedesktop.org/mesa/mesa/commit/?id=9601815b4be886f4d92bf74916de98f3bdb7275c

Are there any plans to enhance the renderers so that at least one of 
them is providing both anisotropic filtering and antialiasing?


As far as I know, anisotropic texture filtering is also one of the 
OpenGL 4.6 requirements.


In 2016 I was told that there are only very few developers involved in 
llvmpipe and that chances are not high that someone is going to port the 
softpipe anisotropic filtering implementation as llvmpipe is much more 
complex. Is there any change in that situation?


If there are no such plans, is there any chance of reverting this commit 
again so that classical OSMesa is available for windows and linux in 
mesa >20?


Regards,

Andreas Fänger

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

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

Re: [Mesa-dev] GLSLstd450NMin/NMax/NClamp

[Brian Paul]

On 11/17/2020 01:59 PM, Brian Paul wrote:

On 11/17/2020 11:45 AM, Ian Romanick wrote:

On 11/17/20 9:25 AM, Brian Paul wrote:


It appears these SPIR-V extension functions don't behave as they should
on Intel (don't know about other Vulkan drivers).

They're supposed to be NaN-aware such that if one argument is NaN, the
other argument is returned.  From our testing, it looks like NMax works
as expected, but not NMin or NClamp.


Do you have some specific test cases that fail?


Yeah, but they come from WHCK and I don't have a small test case.

I'll try your patch series and see what happens.

-Brian


I'm using your patch and I'm also making use of 
VK_KHR_shader_float_control but neither has helped.  Though, the later 
does help with some issues in Nvidia.


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

Re: [Mesa-dev] SpvOpSelect w/ float operands

[Brian Paul]

On 11/18/2020 02:49 AM, Connor Abbott wrote:

On Tue, Nov 17, 2020 at 9:56 PM Brian Paul  wrote:


On 11/17/2020 11:49 AM, Ian Romanick wrote:

On 11/17/20 9:25 AM, Brian Paul wrote:


Using the Intel Vulkan driver, we've found some cases where SpvOpSelect
is returning -0.0 (negative zeros) instead of normal 0.0 depending on
the arguments.


Do you have a specific test case that fails?


Yeah, but as with the NMin/NMax issue it's not a simple test case.  It
comes from a Windows WHCK test suite.



It seems like on some platforms there was an errata about the version of
the SEL instruction that is used for min or max that can return the
wrong signed zero in some cases.

It's also possible that some optimizations are causing problems.  I
don't remember exactly how it works in SPIR-V, but does marking those
SPIR-V instructions as precise (that's what it was in GLSL) make a
difference?


AFAIK, there's only a SPIR-V decoration for tagging things for relaxed
precision.


The SPIR-V equivalent of "precise" is NoContract. But for emulating DX
stuff you're supposed to use KHR_shader_float_controls which was
specifically designed for emulating DX floating-point requirements. In
NIR that just marks everything as "exact" if you force correct
NaN/signed zero/Inf handling.


Thanks for the tip!  I'll take a look at that.

-Brian

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

[Mesa-dev] fp/int64 on gen11/12?

[Brian Paul]

Another Intel question:  It looks like gen11/gen12 don't have fp/int64 
enabled in the Vulkan driver. From gen_device_info.c:


#define GEN11_FEATURES(_gt, _slices, _subslices, _l3) \
   GEN8_FEATURES, \
   GEN11_HW_INFO, \
   .has_64bit_float = false,  \
   .has_64bit_int = false,
...

#define GEN12_FEATURES(_gt, _slices, _l3)   \
   GEN8_FEATURES,   \
   GEN12_HW_INFO,   \
   .has_64bit_float = false,\
   .has_64bit_int = false,

But gen8/9 do support it.  Is this a driver and/or hardware issue?

-Brian

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

Re: [Mesa-dev] GLSLstd450NMin/NMax/NClamp

[Brian Paul]

On 11/17/2020 11:45 AM, Ian Romanick wrote:

On 11/17/20 9:25 AM, Brian Paul wrote:


It appears these SPIR-V extension functions don't behave as they should
on Intel (don't know about other Vulkan drivers).

They're supposed to be NaN-aware such that if one argument is NaN, the
other argument is returned.  From our testing, it looks like NMax works
as expected, but not NMin or NClamp.


Do you have some specific test cases that fail?


Yeah, but they come from WHCK and I don't have a small test case.

I'll try your patch series and see what happens.

-Brian




Looking at the SPIR-V/nir/intel code it's hard to tell what's going on
and whether these semantics are actually being followed.

Any comments?

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=04%7C01%7Cbrianp%40vmware.com%7Ca9f0c2e6cfa846f3b20408d88b28f40d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637412355305449546%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=dWV4%2Bsq4B7xBiCxRmyTj7zM4b2Xfl%2BLjtdll6qHhUAg%3Dreserved=0




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

Re: [Mesa-dev] SpvOpSelect w/ float operands

[Brian Paul]

On 11/17/2020 11:49 AM, Ian Romanick wrote:

On 11/17/20 9:25 AM, Brian Paul wrote:


Using the Intel Vulkan driver, we've found some cases where SpvOpSelect
is returning -0.0 (negative zeros) instead of normal 0.0 depending on
the arguments.


Do you have a specific test case that fails?


Yeah, but as with the NMin/NMax issue it's not a simple test case.  It 
comes from a Windows WHCK test suite.




It seems like on some platforms there was an errata about the version of
the SEL instruction that is used for min or max that can return the
wrong signed zero in some cases.

It's also possible that some optimizations are causing problems.  I
don't remember exactly how it works in SPIR-V, but does marking those
SPIR-V instructions as precise (that's what it was in GLSL) make a
difference?


AFAIK, there's only a SPIR-V decoration for tagging things for relaxed 
precision.


-Brian




I'm wondering if "SpvOpSelect x, a, b" for floats is being implemented
with something like "a*x + b*(1-x)" ?  That might explain where the
negative zeros are coming from.

Our work-around is to implement selection with bitwise operations: (a &
x) | (b & ~x)

It seems to me that SpvOpSelect shouldn't interpret the bits and just
return an exact copy of the argument.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=04%7C01%7Cbrianp%40vmware.com%7C5e1ffabf0f7c47a2f48308d88b298304%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637412357691565798%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=SdzrOYvGGFraVg61OGvIQ6NJrT2i7fye%2Fy03XoZOi7E%3Dreserved=0




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

[Mesa-dev] SpvOpSelect w/ float operands

[Brian Paul]

Using the Intel Vulkan driver, we've found some cases where SpvOpSelect 
is returning -0.0 (negative zeros) instead of normal 0.0 depending on 
the arguments.


I'm wondering if "SpvOpSelect x, a, b" for floats is being implemented 
with something like "a*x + b*(1-x)" ?  That might explain where the 
negative zeros are coming from.


Our work-around is to implement selection with bitwise operations: (a & 
x) | (b & ~x)


It seems to me that SpvOpSelect shouldn't interpret the bits and just 
return an exact copy of the argument.


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

[Mesa-dev] GLSLstd450NMin/NMax/NClamp

[Brian Paul]

It appears these SPIR-V extension functions don't behave as they should 
on Intel (don't know about other Vulkan drivers).


They're supposed to be NaN-aware such that if one argument is NaN, the 
other argument is returned.  From our testing, it looks like NMax works 
as expected, but not NMin or NClamp.


Looking at the SPIR-V/nir/intel code it's hard to tell what's going on 
and whether these semantics are actually being followed.


Any comments?

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

Re: [Mesa-dev] llvmpipe is OpenGL 4.5 conformant.

[Brian Paul]

Nice work!  It's good to see llvmpipe being maintained and improved like 
that.


-Brian

On 10/30/2020 02:24 PM, Dave Airlie wrote:

Just to let everyone know, a month ago I submitted the 20.2 llvmpipe
driver for OpenGL 4.5 conformance under the SPI/X.org umbrella, and it
is now official[1].

Thanks to everyone who helped me drive this forward, and to all the
contributors both to llvmpipe and the general Mesa stack that enabled
this.

Big shout out to Roland Scheidegger for helping review the mountain of
patches I produced in this effort.

My next plans involved submitting lavapipe for Vulkan 1.0, it's at 99%
or so CTS, but there are line drawing, sampler accuracy and some snorm
blending failure I have to work out.
I also ran the OpenCL 3.0 conformance suite against clover/llvmpipe
yesterday and have some vague hopes of driving that to some sort of
completion.

(for GL 4.6 only texture anisotropy is really missing, I've got
patches for SPIR-V support, in case someone was feeling adventurous).

Dave.

[1] 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.khronos.org%2Fconformance%2Fadopters%2Fconformant-products%2Fopengl%23submission_272data=04%7C01%7Cbrianp%40vmware.com%7C9869322310dc466d3a6108d87d11db3a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637396862939563580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=3veHyDPNBvaJjOnvFRdg%2FGtkk3IZuvTFgDxYA3ihONE%3Dreserved=0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=04%7C01%7Cbrianp%40vmware.com%7C9869322310dc466d3a6108d87d11db3a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637396862939563580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=2KyGMO5H%2FodTRaQAxEJlNiVa2HOUkKUKim1sIXBWGUI%3Dreserved=0



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

Re: [Mesa-dev] Gallium interface rename proposals

[Brian Paul]

This all sounds find to me, FWIW.

-Brian


On 09/19/2020 04:24 AM, Marek Olšák wrote:

Hi,

I don't know if you have been following gitlab, but there are a few 
cleanups that I have been considering doing.


Rename PIPE_TRANSFER flags to PIPE_MAP, and pipe_transfer_usage to 
pipe_map_flags:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5749 



Other proposed renames:

transfer_map -> resource_map
transfer_unmap -> resource_unmap
transfer_flush_region -> resource_flush_mapped_range
draw_vbo -> draw

pipe_transfer_* aux helpers -> pipe_resource_* or pipe_texture_* 
depending on context. We already have pipe_buffer_map.


I'm inclined to keep the struct pipe_transfer name unchanged to indicate 
that mappings can cause internal copies.


Please let me know your preferences.

Thanks,
Marek


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Cbrianp%40vmware.com%7Cbedfd0817f704e06e75808d85c863fb3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637361078970597391sdata=qI2%2BFMvRDEIePjAOjbGVwCOusUxyw6I1wwL7fJb%2Blgs%3Dreserved=0



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

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On Sun, Jun 14, 2020 at 9:28 AM Daniel Stone  wrote:

> Hi,
>
> On Fri, 29 May 2020 at 10:08, Erik Faye-Lund
>  wrote:
> > In the light of the explanation above, do you still have objections to
> > this split?
> >
> > Obviously, we haven't moved forward yet ;-)
>
> Well, we have now after getting some agreement. Please enjoy your
> shiny new https://www.mesa3d.org and https://docs.mesa3d.org
> experience.
>
> Brian, could you please change the DNS for mesa3d.org and
> www.mesa3d.org to point to 35.227.58.183? No need to change
> archive.mesa3d.org, that should still point to the old site.
>

Done (may take 3 hours to go into effect).

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

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On 05/13/2020 03:13 AM, Erik Faye-Lund wrote:

On Tue, 2020-05-12 at 12:17 +0200, Erik Faye-Lund wrote:

On Thu, 2020-05-07 at 11:03 -0600, Brian Paul wrote:

On 05/07/2020 04:33 AM, Erik Faye-Lund wrote:

Hey Brian

TLDR; are you OK with me moving forward with the rework of
mesa3d.org?


Yes...



Cool! We've now set up a repo here:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa3d.orgdata=02%7C01%7Cbrianp%40vmware.com%7Cb8ff6d838d46414812f208d7f71df219%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637249580306260765sdata=CLd07g8WPm7mbsa3033pnhycK25xPD5%2Bi00V0MJ6TIY%3Dreserved=0

I pushed the bare minimum (ish) there so far, and sent a MR for the
importing of the news and article-redirects. This is now being served
here:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmesa.pages.freedesktop.org%2Fmesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cb8ff6d838d46414812f208d7f71df219%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637249580306260765sdata=GmaivI1wH4HR6osRoP%2FINlJm%2FgZ6tgb8HsE1ElVhZNE%3Dreserved=0



A few updates:

1. You can now preview the repo with the pending MRs merged here, if
you're interested:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fkusma.pages.freedesktop.org%2Fmesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cb8ff6d838d46414812f208d7f71df219%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637249580306260765sdata=U7cC71x9ql%2FCNXpdNxO2%2But3NAVzDo6S91%2BmevJjMsc%3Dreserved=0

2. Some people have been asking me why the website is set up as a
separate repo rather than a subdirectory inside the main mesa repo.

The quick answer to that is that it seemed more logical that way to me.
The website isn't really tied to the mesa release-schedule, apart from
linking to new releases. It's really its own thing. We might for
instance want to post updates when other projects in the mesa-group
cuts a release. So this seems to give us some more freedom.

If someone has good arguments against this, I'm open to changing it.
But this seems like the best option to me.


I'd really like to keep the Mesa content as part of the main Mesa repo. 
I didn't realize you did that.  The website is part of the project and 
it's more convenient to have it all in one place.


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

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On Tue, May 12, 2020 at 4:04 AM Daniel Stone  wrote:

> Hi Brian,
>
> On Fri, 8 May 2020 at 15:30, Brian Paul  wrote:
> > Done.  easydns says it may take up to 3 hours to go into effect.
>
> Thanks for doing this! Could you please add the following TXT records
> as well (note that they're FQDN, so you might want to chop the
> trailing '.mesa3d.org' from the first segment:
> _gitlab-pages-verification-code.mesa3d.org TXT
> gitlab-pages-verification-code=e8a33eb47c034b08afd339cb3a801d88
> _gitlab-pages-verification-code.www.mesa3d.org TXT
> gitlab-pages-verification-code=28d457c44c75d4a1e460243ded2b4311
> _gitlab-pages-verification-code.docs.mesa3d.org TXT
> gitlab-pages-verification-code=6045fd12252cc01f5891fa3f81b140fe
>

Done.

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

Re: [Mesa-dev] RFC: Memory allocation on Mesa

[Brian Paul]

On 05/11/2020 10:13 AM, Jose Fonseca wrote:

Hi,

To give everybody a bit of background context, this email comes from 
https://gitlab.freedesktop.org/mesa/mesa/-/issues/2911 .


The short story is that Gallium components (but not Mesa) used to have 
their malloc/free calls intercepted, to satisfy certain needs: 1) memory 
debugging on Windows, 2) memory accounting on embedded systems.  But 
with the unification of Gallium into Mesa, the gallium vs non-gallium 
division got blurred, leading to some mallocs being intercepted but not 
the respective frees, and vice-versa.



I admit that trying to intercept mallocs/frees for some components and 
not others is error prone.  We could get this going on again, it's 
doable, but it's possible it would keep come causing troubles, for us or 
others, over and over again.



The two needs mentioned above were mostly VMware's needs.  So I've 
reevaluated, and I /think/ that with some trickery we satisfy those two 
needs differently.  (Without wide spread source code changes.)



On the other hand, VMware is probably not the only one to have such 
needs.  In fact Vulkan spec added memory callbacks precisely with the 
same use cases as ours, as seen 
https://www.khronos.org/registry/vulkan/specs/1.2/html/chap10.html#memory-host which 
states:


/Vulkan provides applications the opportunity to perform host memory
allocations on behalf of the Vulkan implementation. If this feature
is not used, the implementation will perform its own memory
allocations. Since most memory allocations are off the critical
path, this is not meant as a performance feature. *Rather, this can
be useful for certain embedded systems, for debugging purposes (e.g.
putting a guard page after all host allocations), or for memory
allocation logging.*/


And I know there were people interested in having Mesa drivers on 
embedded devices on the past (the old Tunsten Graphics having even been 
multiple times hired to do so), and I'm pretty sure they exist again.




Therefore, rather than shying away from memory allocation abstractions 
now, I wonder if now it's not the time to actually double down on them 
and ensure we do so comprehensively throughout the whole mesa, all drivers?

*
*
After all Mesa traditionally always had MALLOC*/CALLOC*/FREE wrappers 
around malloc/free.  As so many other projects do.




More concretely, I'd like to propose that we:

  * ensure all components use MALLOC*/CALLOC*/FREE and never
malloc/calloc/free directly (unless interfacing with a 3rd party
which expects memory to be allocated/freed with malloc/free directly)
  * Perhaps consider renaming MALLOC -> _mesa_malloc etc while we're at it
  * introduce a mechanisms to quickly catch any mistaken use of
malloc/calloc/free, regardless compiler/OS used:
  o #define malloc/free/calloc as malloc_do_not_use/free_do_not_use
to trigger compilation errors, except on files which explicely
opt out of this (source files which need to interface with 3rd
party, or source files which implement the callbacks)
  o Add a cookie to MALLOC/CALLOC/FREE memory to ensure it's not
inadvertently mixed with malloc/calloc/free

The end goal is that we should be able to have a memory allocation 
abstraction which can be used for all the needs above: memory debugging, 
memory accounting, and satisfying Vulkan host memory callbacks.



Some might retort: why not just play some tricks with the linker, and 
intercept all malloc/free calls, without actually having to modify any 
source code?


Yes, that's indeed technically feasible.  And is precisely the sort of 
trick I was planing to resort to satisfy VMware needs without having to 
further modify the source code.  But for these tricks to work, it is 
absolutely /imperative/ that one statically links C++ library and STL.  
The problem being that if one doesn't then there's an imbalance: the 
malloc/free/new/delete calls done in inline code on C++ headers will be 
intercepted, where as malloc/free/new/delete calls done in code from the 
shared object which is not inlined will not, causing havoc.  This is OK 
for us VMware (we do it already for many other reasons, including 
avoiding DLL hell.)  But I doubt it will be palatable for everybody 
else, particularly Linux distros, to have everything statically linked.


So effectively, if one really wants to implement Vulkan host memory 
callbacks, the best way is to explicitly use malloc/free abstractions, 
instead of the malloc/free directly.



So before we put more time on pursuing either the "all" or "nothing" 
approaches, I'd like to get a feel for where people's preferences are.


Jose




I was tinkering with this on Friday.  My initial idea is to use an 
opt-in approach for memory tracking/debugging.  That is, where we care 
about tracking/debugging we use explicit alternates to malloc/free/etc.


malloc/free/MALLOC/CALLOC_STRUCT/etc are used in thousands of places 

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On Thu, May 7, 2020 at 12:16 PM Brian Paul  wrote:

> On 05/07/2020 11:35 AM, Daniel Stone wrote:
> > Hi,
> >
> > On Thu, 7 May 2020 at 18:08, Erik Faye-Lund
> >  wrote:
> >> On Thu, 2020-05-07 at 11:03 -0600, Brian Paul wrote:
> >>> It seems like only the front page is served at the moment.  Is it
> >>> possible to get a look at the rest?  The front page looks nice.
> >>
> >> Yeah, we need to set up docs.mesa3d.org first. I haven't bothered
> >> setting up the temporary URLs yet, but my current preview is hosted
> >> here:
> >
> > After the nginx infrastructure work over the past few weeks, we can
> > now actually set up redirects. So I think the better thing would be to
> > assume this will be
> https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.mesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cca45e2189c2847b0630f08d7f2ad385c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244698116403831sdata=2WRscmbHCqteoeEcy010Zg%2FxSPXfaoyfwr9MzEOdTL0%3Dreserved=0
> hosted on Pages, then have
> > redirects over for tarball downloads.
> >
> > Brian, could you please add a DNS entry for archive.mesa3d.org as a
> > CNAME to annarchy.freedesktop.org, and docs.mesa3d.org as a CNAME to
> > gitlab.freedesktop.org? If you can do that, I'll get the archive
> > hosting set up separately, then when we're ready we can cut over
> >
> https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.mesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cca45e2189c2847b0630f08d7f2ad385c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244698116403831sdata=2WRscmbHCqteoeEcy010Zg%2FxSPXfaoyfwr9MzEOdTL0%3Dreserved=0
> and mesa3d.org to Pages.
>
> Ok, I'll try to do this asap, but it may be 24 hours.
>

Done.  easydns says it may take up to 3 hours to go into effect.

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

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On 05/07/2020 11:35 AM, Daniel Stone wrote:

Hi,

On Thu, 7 May 2020 at 18:08, Erik Faye-Lund
 wrote:

On Thu, 2020-05-07 at 11:03 -0600, Brian Paul wrote:

It seems like only the front page is served at the moment.  Is it
possible to get a look at the rest?  The front page looks nice.


Yeah, we need to set up docs.mesa3d.org first. I haven't bothered
setting up the temporary URLs yet, but my current preview is hosted
here:


After the nginx infrastructure work over the past few weeks, we can
now actually set up redirects. So I think the better thing would be to
assume this will be 
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.mesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cca45e2189c2847b0630f08d7f2ad385c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244698116403831sdata=2WRscmbHCqteoeEcy010Zg%2FxSPXfaoyfwr9MzEOdTL0%3Dreserved=0
 hosted on Pages, then have
redirects over for tarball downloads.

Brian, could you please add a DNS entry for archive.mesa3d.org as a
CNAME to annarchy.freedesktop.org, and docs.mesa3d.org as a CNAME to
gitlab.freedesktop.org? If you can do that, I'll get the archive
hosting set up separately, then when we're ready we can cut over
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.mesa3d.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7Cca45e2189c2847b0630f08d7f2ad385c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244698116403831sdata=2WRscmbHCqteoeEcy010Zg%2FxSPXfaoyfwr9MzEOdTL0%3Dreserved=0
 and mesa3d.org to Pages.


Ok, I'll try to do this asap, but it may be 24 hours.

-Brian


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

Re: [Mesa-dev] New Mesa3D.org website proposal

[Brian Paul]

On 05/07/2020 04:33 AM, Erik Faye-Lund wrote:

Hey Brian

TLDR; are you OK with me moving forward with the rework of mesa3d.org?


Yes...

(BTW, sorry about the URL mangling below)



As you hopefully are aware of, I've been working on a new website for
mesa3d.org, split into a "marketing"-frontpage and a documentation
page.

You can read more about the structure and details here if you haven't
already:
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F4630data=02%7C01%7Cbrianp%40vmware.com%7C6998774738b349bcb79108d7f2720ca6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63723970864799sdata=1Z%2B3KKoVjNLHRvIf2VbK7KHSM4kh10fZQ1zdfPsguA0%3Dreserved=0

That MR only deals with the marketing website, so I think it's time to
talk more about the main website. What I currently have is located
here:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fkusma%2Fkusma.pages.freedesktop.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7C6998774738b349bcb79108d7f2720ca6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63723970864799sdata=ttMgZuWT%2Bgg19H9qx1XmUQgl%2FO%2FlFF3vZuGkKxQGC9g%3Dreserved=0

This builds on CI and currently gets served here:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fkusma.pages.freedesktop.org%2Fdata=02%7C01%7Cbrianp%40vmware.com%7C6998774738b349bcb79108d7f2720ca6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63723970864799sdata=258fUaF1DrIioGS1VTROkJznMNjI3xhxmbdl9RWyJZU%3Dreserved=0


It seems like only the front page is served at the moment.  Is it 
possible to get a look at the rest?  The front page looks nice.






Since last time I published any details about this, I've:
1. Switched the static-site-generator from Jekyll to Hugo. This lead to
a build-time decrease from about two minutes to about 200ms on my
machine. This makes iteration times a lot more bearable.
2. Back-paddled on moving some pages. I think we can hold off on moving
pages to the website until the initial work lands.
3. Gotten approval from the Khronos marketing team about the usage of
their logos / trademarks. They had some minor requests for changes,
which has been taken into account.

The news articles and redirects for URL compatibility is converted from
a branch in my Mesa fork, here:

https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fkusma%2Fmesa%2F-%2Ftree%2Fdocs-convert-news-hugodata=02%7C01%7Cbrianp%40vmware.com%7C6998774738b349bcb79108d7f2720ca6%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C63723970864799sdata=rmXuKgyMFAZqJD4ij%2F8mR1EvA4HvFYwnR5Gd%2BCdgJFM%3Dreserved=0

So, after a long introduction (sorry if a lot of this is stuff you
already know), what I really want is for you Brian, as the project
originator and listed webmaster, to say how you feel about all of this.


Please remove me as the webmaster.  I still occasionally get emails from 
that.  Let readers know how to submit changes/edits.




If you're happy to move forward with this, I will start the technical
discussions with the FDO admins and other people who might need to be
involved in setting this up for production.


Yeah, go ahead.  Sorry for not monitoring this work.  I basically have 
zero time for Mesa nowadays.


-Brian

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

[Mesa-dev] Windows build broken

[Brian Paul]

by:

commit f8f1413070ae079443ab31a75679cfd10cb756ed
Author: Pierre-Eric Pelloux-Prayer 
Date:   Mon Mar 16 10:49:17 2020 +0100

util/u_process: add util_get_process_exec_path

Reviewed-by: Marek Olšák 
Part-of: 




scons: Building targets ...
  Compiling src\gallium\auxiliary\driver_ddebug\dd_draw.c ...
dd_draw.c
  Compiling src\gallium\auxiliary\os\os_process.c ...
os_process.c
  Compiling src\util\os_file.c ...
os_file.c
  Compiling src\util\u_process.c ...
u_process.c
  Compiling src\util\u_queue.c ...
src\util\u_process.c(34): fatal error C1083: Cannot open include file: 
'unistd.h': No such file or directory



The #include of unistd.h should probably be moved into the later #if 
DETECT_OS_WINDOWS block.


-Brian

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

Re: [Mesa-dev] Drop scons for 20.1?

[Brian Paul]

I'm on vacation this week. When I get back next week we'll talk about this
again at VMware. Unfortunately, integrating a new build tool into our
infrastructure here can be pretty difficult.  But I totally understand the
appeal of getting rid of SCons.  Please hold on a bit...

-Brian



On Wed, Feb 26, 2020 at 2:04 PM Kristian Høgsberg 
wrote:

> On Wed, Feb 26, 2020 at 12:16 PM Jose Fonseca  wrote:
> >
> > > but it bothers me how we keep not making a decision on this. If we'd
> said, "let's keep it and support it", that would something.
> >
> > I'm surprised there's any doubt.
> >
> > SCons works great for us.   Meson gives no immediate benefit for us
> other than headaches.  If we cared about nothing but ourselves, we'd keep
> SCons indefinitely, until it became a pain.
> >
> > The only reason we don't stubbornly put the foot down is that we
> understand that having one single build system would be beneficial the
> whole community, and of course we appreciate all the work Dylan and others
> did to get Meson to work on Windows, so we'd like to get there one day.
> >
> > That said, I don't understand why the rest of the Mesa community putting
> a gun against our head to abandon SCons.
> >
> > Aren't we maintaining the SCons build?  Since when in Mesa community are
> some entitled to start remove code that still works, is used, and
> maintained by others
>
> Nobody is entitled to remove the code, that's why we're having this
> discussion. And I bet it's frustrating for you to have to deal with
> this again and again, but on the other side, it's frustrating to see
> this issue come up again and again with no evident progress
> whatsoever. What has happened on your side since the last time this
> was discussed? When is that "one day"? How are we supposed to move
> this forward without bringing it up?
>
> As for removing code that works - we do that All. The. Time. We
> refactor subsystems, IRs, entire drivers get rewritten and replace the
> working code that was there before, in order the lower the maintenance
> burden, run across more platforms, shader a compiler pass, reduce
> techincal debt etc.
>
> Kristian
>
> >
> > Jose
> >
> > 
> > From: Kristian Høgsberg 
> > Sent: Wednesday, February 26, 2020 18:37
> > To: Jason Ekstrand 
> > Cc: Rob Clark ; mesa-dev <
> mesa-dev@lists.freedesktop.org>; Dylan Baker ;
> Jose Fonseca ; Brian Paul 
> > Subject: Re: [Mesa-dev] Drop scons for 20.1?
> >
> > On Tue, Feb 25, 2020 at 8:15 PM Jason Ekstrand 
> wrote:
> > >
> > > +Jose & Brian
> > >
> > > I'm not personally opposed but I also can't remember the last time I
> had to
> > > fix the scons build. I think it's been years. Maybe that's because I
> don't
> > > work on GL anymore? In any case, I don't know that it's really costing
> us
> > > that much given that basically none of the drivers actually build with
> it.
> > > But fat meh, I guess.
> >
> > Maybe it is a bit meh... I did the MR to remove SCons and it's smaller
> > that I thought it would be:
> >
> >
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F3955data=02%7C01%7Cjfonseca%40vmware.com%7C6b2e8f2abc98458d18ad08d7baeb0443%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637183390863817583sdata=96lM4flW9ja6fJG95nlNdmftNiYpajxpg0Il850%2FDLk%3Dreserved=0
> >
> > but it bothers me how we keep not making a decision on this. If we'd
> > said, "let's keep it and support it", that would something. But
> > whenever it comes up, Dylan maybe fixes something on the windows
> > build, we talk about trying to switch Windows to meson and then...
> > nothing.
> >
> > Also, we've had this unfortunate split between Linux and Windows build
> > systems where autotools suck on Windows and nobody on Unix ever had a
> > reason to use SCons.  With meson we've picked something that's a
> > legitimate improvement on both sides, get's us back to one build
> > system and done more than due dilligence to make it work on Windows
> > and we're not taking the last step because... meh?
> >
> > Kristian
> >
> > > --Jason
> > >
> > > On February 25, 2020 21:56:30 Rob Clark  wrote:
> > >
> > > > It looks like we have 4 scons build jobs in CI.. I'm not sure how
> much
> > > > that costs us, but I guess those cycles could be put to better use?
> > > > So even ignoring the developer-cycles issue (ie. someone making
> > &g

Re: [Mesa-dev] [Review Request (master branch)] svga: Use pipe_shader_state_from_tgsi to set shader state

[Brian Paul]

I'm going to update the docs regarding patches and gitlab.  It's kind of 
a mess now.


-Brian


On 02/11/2020 03:03 AM, Michel Dänzer wrote:


Hi Charmaine,


it looks like you pushed this patch and another one directly to the main
Mesa repository master branch.

Pushing directly to the main Mesa repository is no longer common
practice, and indeed discouraged, as it circumvents the pre-merge GitLab
CI pipeline and forfeits all other benefits of a merge request (MR).

The common practice is to create an MR (normally already for patch
review) and assign it to Marge Bot for merging. Marge will rebase as
needed and merge once the pre-merge CI pipeline has passed.


Thanks,


On 2020-01-29 5:14 p.m., Neha Bhende wrote:

Use pipe_shader_state_from_tgsi() to set shader state for transformed
shader so that we get all correct data for respective shader state.

This fixes several regressed glretrace, piglit crashes found during merging
upsteam mesa

Fixes: bf12bc2dd7a2 (draw: add nir info gathering and building support)

Reviewed-by: Charmaine Lee 
---
  src/gallium/drivers/svga/svga_state_tgsi_transform.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/svga/svga_state_tgsi_transform.c 
b/src/gallium/drivers/svga/svga_state_tgsi_transform.c
index b567aab6bc8..9d701b73772 100644
--- a/src/gallium/drivers/svga/svga_state_tgsi_transform.c
+++ b/src/gallium/drivers/svga/svga_state_tgsi_transform.c
@@ -131,7 +131,7 @@ emulate_point_sprite(struct svga_context *svga,
   tgsi_dump(new_tokens, 0);
}
  
-  templ.tokens = new_tokens;

+  pipe_shader_state_from_tgsi(, new_tokens);
templ.stream_output.num_outputs = 0;
  
if (streamout) {







___
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?

[Brian Paul]

On 12/11/2019 10:42 AM, Jason Ekstrand wrote:
On Wed, Dec 11, 2019 at 11:33 AM Michel Dänzer <mailto:mic...@daenzer.net>> wrote:


On 2019-12-11 5:47 p.m., Brian Paul wrote:
 >
 > I've had little time for Mesa work the past 18 months.

That makes me sad, I hope you'll have more time again in the future.


Between family life and VMware work, there's little left over.  I'll be 
lurking if nothing else.




 > 1. I don't think the mesa-dev list should be shut down nor purged
from
 > the documentation.

Yeah, I don't think anybody seriously suggested the list should be shut
down, just that GitLab is now preferred for patch submission & review.


 > Someone mentioned hardly reading the mailing list anymore.  I still
 > haven't gotten into the habit of monitoring the MRs page...

Instead of monitoring a web page, I recommend setting up notifications
via the bell icon on https://gitlab.freedesktop.org/mesa/mesa

<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa=02%7C01%7Cbrianp%40vmware.com%7Cb98a969240530d4908d77e61906d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637116829815378870=vENdTPXFgRswVXvIKNhfYf7G3msmOTYYcJ2MAvU%2BKxc%3D=0>
[0]. If
you select "Custom", you can select in detail which events you want to
get notification e-mails for. Check "New merge request" to get an e-mail
for each new MR created. Then you can either enable notifications for
other MR events here as well, or enable all notifications for MRs you're
interested in using the "Notifications" switch in the right hand panel
on each MR's page.


Thanks for the tip!  That's the kind of thing that would be useful to 
document.



You can also subscribe to specific labels which is what I've done.  That 
way I get e-mails about anything going on in NIR or our Vulkan driver 
but don't have to see every nouveau MR.


Are labels put on MRs automatically or does the person submitting have 
to do that manually?


-Brian
___
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?

[Brian Paul]

On 12/09/2019 04: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've had little time for Mesa work the past 18 months.  But I'd say:

1. I don't think the mesa-dev list should be shut down nor purged from 
the documentation.


2. I think the complete workflow for submitting patches via gitlab MRs 
should be documented on the Mesa site.  Being a gitlab newbie I wasted a 
lot of time a few months ago trying to figure out the details.  Mesa's 
"Submitting Patches" page is kind of a mess.  It would be great if 
someone could work on that.


Someone mentioned hardly reading the mailing list anymore.  I still 
haven't gotten into the habit of monitoring the MRs page...


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

Re: [Mesa-dev] [PATCH] util/atomic: Fix p_atomic_add for unlocked and msvc paths

[Brian Paul]

Reviewed-by: Brian Paul 

On 12/09/2019 10:49 AM, srol...@vmware.com wrote:

From: Roland Scheidegger 

Braces mismatch (flagged by CI, untested).

Fixes: 385d13f26d2 "util/atomic: Add a _return variant of p_atomic_add"
---
  src/util/u_atomic.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/u_atomic.h b/src/util/u_atomic.h
index 9cbc6dd1eaa..1ad87c8feb1 100644
--- a/src/util/u_atomic.h
+++ b/src/util/u_atomic.h
@@ -89,7 +89,7 @@
  #define p_atomic_dec_zero(_v) (p_atomic_dec_return(_v) == 0)
  #define p_atomic_inc(_v) ((void) p_atomic_inc_return(_v))
  #define p_atomic_dec(_v) ((void) p_atomic_dec_return(_v))
-#define p_atomic_add(_v, _i) ((void) p_atomic_add_return((_v), (_i))
+#define p_atomic_add(_v, _i) ((void) p_atomic_add_return((_v), (_i)))
  #define p_atomic_inc_return(_v) (++(*(_v)))
  #define p_atomic_dec_return(_v) (--(*(_v)))
  #define p_atomic_add_return(_v, _i) (*(_v) = *(_v) + (_i))
@@ -146,7 +146,7 @@
   (assert(!"should not get here"), 0))
  
  #define p_atomic_add(_v, _i) \

-   ((void) p_atomic_add_return((_v), (_i))
+   ((void) p_atomic_add_return((_v), (_i)))
  
  #define p_atomic_add_return(_v, _i) (\

 sizeof *(_v) == sizeof(char)? _InterlockedExchangeAdd8 ((char *)   
(_v), (_i)) : \



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

Re: [Mesa-dev] [PATCH] nir: no-op C99 _Pragma() with MSVC

[Brian Paul]

On 11/22/2019 09:16 PM, Ian Romanick wrote:

On 11/22/19 6:49 PM, Brian Paul wrote:

This fixes a build failure on MSVC.

BTW, it looks like clang supports _Pragma() but I don't know if it
understands the "gcc unroll N" directive.


It probably doesn't, but that should be okay.  This just exists to speed
up the debug builds in the pre-merge CI.

Reviewed-by: Ian Romanick 




BTW, With gcc 5.4.0, there's a lot of warnings about the pragma not 
being understood:


../src/compiler/nir/nir_range_analysis.c:268:0: warning: ignoring 
#pragma GCC unroll [-Wunknown-pragmas]

ASSERT_TABLE_IS_COMMUTATIVE(union_table);
 ^
../src/compiler/nir/nir_range_analysis.c:268:0: warning: ignoring 
#pragma GCC unroll [-Wunknown-pragmas]
../src/compiler/nir/nir_range_analysis.c:269:0: warning: ignoring 
#pragma GCC unroll [-Wunknown-pragmas]


I'd have to dig to see what version of gcc added that to add more 
preprocessor tests to silence it.


-Brian



Signed-off-by: Brian Paul 
---
  src/compiler/nir/nir_range_analysis.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/src/compiler/nir/nir_range_analysis.c 
b/src/compiler/nir/nir_range_analysis.c
index df5d4da..d38bcc0 100644
--- a/src/compiler/nir/nir_range_analysis.c
+++ b/src/compiler/nir/nir_range_analysis.c
@@ -218,6 +218,13 @@ analyze_constant(const struct nir_alu_instr *instr, 
unsigned src,
   */
  #define ___ unknown
  
+

+/* MSVC doesn't have C99's _Pragma() */
+#ifdef _MSC_VER
+#define _Pragma(x)
+#endif
+
+
  #ifndef NDEBUG
  #define ASSERT_TABLE_IS_COMMUTATIVE(t)\
 do {   \





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

[Mesa-dev] [PATCH] nir: no-op C99 _Pragma() with MSVC

[Brian Paul]

This fixes a build failure on MSVC.

BTW, it looks like clang supports _Pragma() but I don't know if it
understands the "gcc unroll N" directive.

Signed-off-by: Brian Paul 
---
 src/compiler/nir/nir_range_analysis.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/compiler/nir/nir_range_analysis.c 
b/src/compiler/nir/nir_range_analysis.c
index df5d4da..d38bcc0 100644
--- a/src/compiler/nir/nir_range_analysis.c
+++ b/src/compiler/nir/nir_range_analysis.c
@@ -218,6 +218,13 @@ analyze_constant(const struct nir_alu_instr *instr, 
unsigned src,
  */
 #define ___ unknown
 
+
+/* MSVC doesn't have C99's _Pragma() */
+#ifdef _MSC_VER
+#define _Pragma(x)
+#endif
+
+
 #ifndef NDEBUG
 #define ASSERT_TABLE_IS_COMMUTATIVE(t)\
do {   \
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH] Call shmget() with permission 0600 instead of 0777

[Brian Paul]

Ping again.


On 10/24/2019 03:25 PM, Brian Paul wrote:

Ping.  Anyone?

-Brian

On Tue, Oct 22, 2019 at 3:52 PM Brian Paul <mailto:bri...@vmware.com>> wrote:


A security advisory (TALOS-2019-0857/CVE-2019-5068) found that
creating shared memory regions with permission mode 0777 could allow
any user to access that memory.  Several Mesa drivers use shared-
memory XImages to implement back buffers for improved performance.

This path changes the shmget() calls to use 0600 (user r/w).

Tested with legacy Xlib driver and llvmpipe.

Cc: mesa-sta...@lists.freedesktop.org
<mailto:mesa-sta...@lists.freedesktop.org>
---
  src/gallium/winsys/sw/dri/dri_sw_winsys.c   | 3 ++-
  src/gallium/winsys/sw/xlib/xlib_sw_winsys.c | 3 ++-
  src/mesa/drivers/x11/xm_buffer.c            | 3 ++-
  3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gallium/winsys/sw/dri/dri_sw_winsys.c
b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
index 761f5d1..2e5970b 100644
--- a/src/gallium/winsys/sw/dri/dri_sw_winsys.c
+++ b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
@@ -92,7 +92,8 @@ alloc_shm(struct dri_sw_displaytarget *dri_sw_dt,
unsigned size)
  {
     char *addr;

-   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
+   /* 0600 = user read+write */
+   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
     if (dri_sw_dt->shmid < 0)
        return NULL;

diff --git a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
index c14c9de..edebb48 100644
--- a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
+++ b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
@@ -126,7 +126,8 @@ alloc_shm(struct xlib_displaytarget *buf,
unsigned size)
     shminfo->shmid = -1;
     shminfo->shmaddr = (char *) -1;

-   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
+   /* 0600 = user read+write */
+   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
     if (shminfo->shmid < 0) {
        return NULL;
     }
diff --git a/src/mesa/drivers/x11/xm_buffer.c
b/src/mesa/drivers/x11/xm_buffer.c
index d945d8a..0da08a6 100644
--- a/src/mesa/drivers/x11/xm_buffer.c
+++ b/src/mesa/drivers/x11/xm_buffer.c
@@ -89,8 +89,9 @@ alloc_back_shm_ximage(XMesaBuffer b, GLuint width,
GLuint height)
        return GL_FALSE;
     }

+   /* 0600 = user read+write */
     b->shminfo.shmid = shmget(IPC_PRIVATE,
b->backxrb->ximage->bytes_per_line
-                            * b->backxrb->ximage->height,
IPC_CREAT|0777);
+                             * b->backxrb->ximage->height,
IPC_CREAT | 0600);
     if (b->shminfo.shmid < 0) {
        _mesa_warning(NULL, "shmget failed while allocating back
buffer.\n");
        XDestroyImage(b->backxrb->ximage);
-- 
1.8.5.6


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

<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev=02%7C01%7Cbrianp%40vmware.com%7Cae41e27c807a41901c9308d758c8b6f7%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637075491402263356=FDvN5Y%2BHswpYAfg96qF9sDykW7nn9ubkedWkFTKQTU0%3D=0>



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

[Mesa-dev] [PATCH] spirv: s/{}/{0}/ initializer to fix MSVC build

[Brian Paul]

Trivial.
---
 src/compiler/spirv/vtn_variables.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index 944d1f0..37ad4f2 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -50,7 +50,7 @@ static struct vtn_pointer*
 vtn_decorate_pointer(struct vtn_builder *b, struct vtn_value *val,
  struct vtn_pointer *ptr)
 {
-   struct vtn_pointer dummy = { };
+   struct vtn_pointer dummy = {0};
vtn_foreach_decoration(b, val, ptr_decoration_cb, );
 
/* If we're adding access flags, make a copy of the pointer.  We could
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] nir: fix a couple signed/unsigned comparison warnings in nir_builder.h

[Brian Paul]

---
 src/compiler/nir/nir_builder.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h
index de00fe7..aed4759 100644
--- a/src/compiler/nir/nir_builder.h
+++ b/src/compiler/nir/nir_builder.h
@@ -782,7 +782,7 @@ nir_extract_bits(nir_builder *b, nir_ssa_def **srcs, 
unsigned num_srcs,
for (unsigned i = 0; i < num_srcs; i++)
   common_bit_size = MIN2(common_bit_size, srcs[i]->bit_size);
if (first_bit > 0)
-  common_bit_size = MIN2(common_bit_size, (1 << (ffs(first_bit) - 1)));
+  common_bit_size = MIN2(common_bit_size, (1u << (ffs(first_bit) - 1)));
 
/* We don't want to have to deal with 1-bit values */
assert(common_bit_size >= 8);
@@ -800,7 +800,7 @@ nir_extract_bits(nir_builder *b, nir_ssa_def **srcs, 
unsigned num_srcs,
   const unsigned bit = first_bit + (i * common_bit_size);
   while (bit >= src_end_bit) {
  src_idx++;
- assert(src_idx < num_srcs);
+ assert(src_idx < (int) num_srcs);
  src_start_bit = src_end_bit;
  src_end_bit += srcs[src_idx]->bit_size *
 srcs[src_idx]->num_components;
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] s/APIENTRY/GLAPIENTRY/ in teximage.c

[Brian Paul]

The later is the right symbol for entrypoint functions.
---
 src/mesa/main/texgetimage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index e43f336..d6ec4c5 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -1969,7 +1969,7 @@ _mesa_GetCompressedTextureImage(GLuint texture, GLint 
level,
 }
 
 
-void APIENTRY
+void GLAPIENTRY
 _mesa_GetCompressedTextureSubImage(GLuint texture, GLint level,
GLint xoffset, GLint yoffset,
GLint zoffset, GLsizei width,
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH] Call shmget() with permission 0600 instead of 0777

[Brian Paul]

Ping.  Anyone?

-Brian

On Tue, Oct 22, 2019 at 3:52 PM Brian Paul  wrote:

> A security advisory (TALOS-2019-0857/CVE-2019-5068) found that
> creating shared memory regions with permission mode 0777 could allow
> any user to access that memory.  Several Mesa drivers use shared-
> memory XImages to implement back buffers for improved performance.
>
> This path changes the shmget() calls to use 0600 (user r/w).
>
> Tested with legacy Xlib driver and llvmpipe.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/gallium/winsys/sw/dri/dri_sw_winsys.c   | 3 ++-
>  src/gallium/winsys/sw/xlib/xlib_sw_winsys.c | 3 ++-
>  src/mesa/drivers/x11/xm_buffer.c| 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/winsys/sw/dri/dri_sw_winsys.c
> b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
> index 761f5d1..2e5970b 100644
> --- a/src/gallium/winsys/sw/dri/dri_sw_winsys.c
> +++ b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
> @@ -92,7 +92,8 @@ alloc_shm(struct dri_sw_displaytarget *dri_sw_dt,
> unsigned size)
>  {
> char *addr;
>
> -   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
> +   /* 0600 = user read+write */
> +   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
> if (dri_sw_dt->shmid < 0)
>return NULL;
>
> diff --git a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
> b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
> index c14c9de..edebb48 100644
> --- a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
> +++ b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
> @@ -126,7 +126,8 @@ alloc_shm(struct xlib_displaytarget *buf, unsigned
> size)
> shminfo->shmid = -1;
> shminfo->shmaddr = (char *) -1;
>
> -   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
> +   /* 0600 = user read+write */
> +   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
> if (shminfo->shmid < 0) {
>return NULL;
> }
> diff --git a/src/mesa/drivers/x11/xm_buffer.c
> b/src/mesa/drivers/x11/xm_buffer.c
> index d945d8a..0da08a6 100644
> --- a/src/mesa/drivers/x11/xm_buffer.c
> +++ b/src/mesa/drivers/x11/xm_buffer.c
> @@ -89,8 +89,9 @@ alloc_back_shm_ximage(XMesaBuffer b, GLuint width,
> GLuint height)
>return GL_FALSE;
> }
>
> +   /* 0600 = user read+write */
> b->shminfo.shmid = shmget(IPC_PRIVATE,
> b->backxrb->ximage->bytes_per_line
> -* b->backxrb->ximage->height, IPC_CREAT|0777);
> + * b->backxrb->ximage->height, IPC_CREAT |
> 0600);
> if (b->shminfo.shmid < 0) {
>_mesa_warning(NULL, "shmget failed while allocating back
> buffer.\n");
>XDestroyImage(b->backxrb->ximage);
> --
> 1.8.5.6
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] Call shmget() with permission 0600 instead of 0777

[Brian Paul]

A security advisory (TALOS-2019-0857/CVE-2019-5068) found that
creating shared memory regions with permission mode 0777 could allow
any user to access that memory.  Several Mesa drivers use shared-
memory XImages to implement back buffers for improved performance.

This path changes the shmget() calls to use 0600 (user r/w).

Tested with legacy Xlib driver and llvmpipe.

Cc: mesa-sta...@lists.freedesktop.org
---
 src/gallium/winsys/sw/dri/dri_sw_winsys.c   | 3 ++-
 src/gallium/winsys/sw/xlib/xlib_sw_winsys.c | 3 ++-
 src/mesa/drivers/x11/xm_buffer.c| 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gallium/winsys/sw/dri/dri_sw_winsys.c 
b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
index 761f5d1..2e5970b 100644
--- a/src/gallium/winsys/sw/dri/dri_sw_winsys.c
+++ b/src/gallium/winsys/sw/dri/dri_sw_winsys.c
@@ -92,7 +92,8 @@ alloc_shm(struct dri_sw_displaytarget *dri_sw_dt, unsigned 
size)
 {
char *addr;
 
-   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
+   /* 0600 = user read+write */
+   dri_sw_dt->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
if (dri_sw_dt->shmid < 0)
   return NULL;
 
diff --git a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c 
b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
index c14c9de..edebb48 100644
--- a/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
+++ b/src/gallium/winsys/sw/xlib/xlib_sw_winsys.c
@@ -126,7 +126,8 @@ alloc_shm(struct xlib_displaytarget *buf, unsigned size)
shminfo->shmid = -1;
shminfo->shmaddr = (char *) -1;
 
-   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777);
+   /* 0600 = user read+write */
+   shminfo->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0600);
if (shminfo->shmid < 0) {
   return NULL;
}
diff --git a/src/mesa/drivers/x11/xm_buffer.c b/src/mesa/drivers/x11/xm_buffer.c
index d945d8a..0da08a6 100644
--- a/src/mesa/drivers/x11/xm_buffer.c
+++ b/src/mesa/drivers/x11/xm_buffer.c
@@ -89,8 +89,9 @@ alloc_back_shm_ximage(XMesaBuffer b, GLuint width, GLuint 
height)
   return GL_FALSE;
}
 
+   /* 0600 = user read+write */
b->shminfo.shmid = shmget(IPC_PRIVATE, b->backxrb->ximage->bytes_per_line
-* b->backxrb->ximage->height, IPC_CREAT|0777);
+ * b->backxrb->ximage->height, IPC_CREAT | 0600);
if (b->shminfo.shmid < 0) {
   _mesa_warning(NULL, "shmget failed while allocating back buffer.\n");
   XDestroyImage(b->backxrb->ximage);
-- 
1.8.5.6

___
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

[Brian Paul]

On 09/11/2019 03:06 PM, Ian Romanick wrote:

On 9/10/19 10:53 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 */


Could we get a comment in the definition of gl_nir_linker_options to
remind people to either add options only to the end or double check all
of the places that initialize the structures?  If someone adds 'bool
do_foo_instead_of_bar' option at the beginning of that struct, it will
cause problems.


};
if (!gl_nir_link(ctx, shader_program, ))
   return GL_FALSE;





How about something simple like this instead:


diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker

index d6a0264..4f5acfd 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -687,9 +687,14 @@ st_link_nir(struct gl_context *ctx,
 * st_nir_preprocess.
 */
if (shader_program->data->spirv) {
-  static const gl_nir_linker_options opts = {
- true /*fill_parameters */
-  };
+  /* Note: this object could be static const but designated
+   * initializers are not part of the C++ standard (allowed by GCC
+   * but not MSVC.)
+   */
+  gl_nir_linker_options opts = { 0 };
+
+  opts.fill_parameters = true;
+
   if (!gl_nir_link(ctx, shader_program, ))
  return GL_FALSE;


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

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

[Brian Paul]

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 */
   };
   if (!gl_nir_link(ctx, shader_program, ))
  return GL_FALSE;
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH 4/4] scons: Make GCC builds stricter.

[Brian Paul]

For the series,  Reviewed-by: Brian Paul 

On 08/27/2019 04:57 AM, Jose Fonseca wrote:

Uses some of the same -Werror options used by Meson, as suggested by
Michel Daezer.
---
  scons/gallium.py | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scons/gallium.py b/scons/gallium.py
index 21197c8d0d1..2eff4174257 100755
--- a/scons/gallium.py
+++ b/scons/gallium.py
@@ -473,7 +473,10 @@ def generate(env):
  '-fmessage-length=0', # be nice to Eclipse
  ]
  cflags += [
-'-Wmissing-prototypes',
+'-Werror=implicit-function-declaration',
+'-Werror=missing-prototypes',
+'-Werror=return-type',
+'-Werror=incompatible-pointer-types',
  '-std=gnu99',
  ]
  if icc:



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

Re: [Mesa-dev] [PATCH] st/mesa: don't allocate mipmapped texture for NEAREST_MIPMAP_LINEAR

[Brian Paul]

Reviewed-by: Brian Paul 


On 08/10/2019 10:14 AM, Marek Olšák wrote:

Ping

On Fri., Aug. 2, 2019, 19:13 Marek Olšák, <mailto:mar...@gmail.com>> wrote:


From: Marek Olšák mailto:marek.ol...@amd.com>>

---
  src/mesa/state_tracker/st_cb_texture.c | 12 
  1 file changed, 12 insertions(+)

diff --git a/src/mesa/state_tracker/st_cb_texture.c
b/src/mesa/state_tracker/st_cb_texture.c
index 0edb3ea5c7e..1ace61863ff 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -516,20 +516,32 @@ allocate_full_mipmap(const struct
st_texture_object *stObj,
        return FALSE;

     if (stObj->base.BaseLevel == 0 && stObj->base.MaxLevel == 0)
        return FALSE;

     if (stObj->base.Sampler.MinFilter == GL_NEAREST ||
         stObj->base.Sampler.MinFilter == GL_LINEAR)
        /* not a mipmap minification filter */
        return FALSE;

+   /* If the following sequence of GL calls is used:
+    *   glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB, ...
+    *   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
GL_LINEAR);
+    *
+    * we would needlessly allocate a mipmapped texture, because the
initial
+    * MinFilter is GL_NEAREST_MIPMAP_LINEAR. Catch this case and don't
+    * allocate a mipmapped texture by default. This may cause texture
+    * reallocation later, but GL_NEAREST_MIPMAP_LINEAR is pretty rare.
+    */
+   if (stObj->base.Sampler.MinFilter == GL_NEAREST_MIPMAP_LINEAR)
+      return FALSE;
+
     if (stObj->base.Target == GL_TEXTURE_3D)
        /* 3D textures are seldom mipmapped */
        return FALSE;

     return TRUE;
  }


  /**
   * Try to allocate a pipe_resource object for the given
st_texture_object.
-- 
2.17.1




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

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

Re: [Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8

[Brian Paul]

On 08/02/2019 10:36 AM, srol...@vmware.com wrote:

From: Roland Scheidegger 

These versions still need wrapper but already have both success and
failure ordering.
(Compile tested on llvm 3.7, llvm 3.8.)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=02
---
  src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp 
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 79d10293e80..723c84d57c2 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -822,15 +822,29 @@ static llvm::AtomicOrdering 
mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
 llvm_unreachable("Invalid LLVMAtomicOrdering value!");
  }
  
+#if HAVE_LLVM < 0x305

  LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
  LLVMValueRef Cmp, LLVMValueRef New,
  LLVMAtomicOrdering SuccessOrdering,
  LLVMAtomicOrdering FailureOrdering,
  LLVMBool SingleThread)
  {
-   /* LLVM 3.8 doesn't have a second ordering and uses old 
SynchronizationScope enum */
+   /* LLVM < 3.5 doesn't have a second ordering and uses old 
SynchronizationScope enum */
 return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), 
llvm::unwrap(Cmp),
llvm::unwrap(New), 
mapFromLLVMOrdering(SuccessOrdering),
SingleThread ? 
llvm::SynchronizationScope::SingleThread : 
llvm::SynchronizationScope::CrossThread));
  }
+#else
+LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
+LLVMValueRef Cmp, LLVMValueRef New,
+LLVMAtomicOrdering SuccessOrdering,
+LLVMAtomicOrdering FailureOrdering,
+LLVMBool SingleThread)
+{
+   return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), 
llvm::unwrap(Cmp),
+  llvm::unwrap(New), 
mapFromLLVMOrdering(SuccessOrdering),
+  
mapFromLLVMOrdering(FailureOrdering),
+  SingleThread ? 
llvm::SynchronizationScope::SingleThread : 
llvm::SynchronizationScope::CrossThread));
+}
+#endif
  #endif



Could the #if / #endif logic be moved into the body of 
LLVMBuildAtomicCmpXchg() so the whole function isn't duplicated?


Other than that,
Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision

[Brian Paul]

On 08/02/2019 09:56 AM, Eric Engestrom wrote:

On Friday, 2019-08-02 17:50:17 +0200, Michel Dänzer wrote:

On 2019-08-02 5:37 p.m., Brian Paul wrote:

Ugh, I didn't mean to do this.  I'm trying to figure out how to make a
merge request with gitlab.


Just push to a branch in your personal repository, and the output of git
push contains an URL for creating an MR for it.


Precisely, but just to be extra clear: your personal repo needs to be
forked from the main mesa repo [1], not just "a repo containing the mesa
git history".
GitLab needs to know the two are linked and pressing that "fork" button
is how you tell it.


Yeah, I just figured that out a few minutes ago.  After I figure out all 
the detailed steps from scratch I'll add it to the documentation.


I've really never done anything with gitlab, github, etc. and have been 
busy with non-Mesa work for over a year now.  I have a lot of catch-up 
to do.


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

Re: [Mesa-dev] Mesa (update-reviewers-for-vmware): i965/clear: clear_value better precision

[Brian Paul]

Ugh, I didn't mean to do this.  I'm trying to figure out how to make a 
merge request with gitlab.


-Brian


On 08/02/2019 09:35 AM, GitLab Mirror wrote:

Module: Mesa
Branch: update-reviewers-for-vmware
Commit: a86eccfb78092493b3999849db62613838951756
URL:
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Da86eccfb78092493b3999849db62613838951756data=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=bLEb7XY3mLFAkME7v8QJ0GugG%2FbzFoNONSSGTjKSfFs%3Dreserved=0

Author: Sergii Romantsov 
Date:   Fri Jul 12 16:46:45 2019 +0300

i965/clear: clear_value better precision

Test-case with depth-clear 0.5 and format
MESA_FORMAT_Z24_UNORM_X8_UINT fails due inconsistent
clear-value of 0.497.
Maybe its better to improve?

CC: Jason Ekstrand 
Fixes: 0ae9ce0f29ea (i965/clear: Quantize the depth clear value based on the 
format)
Bugzilla: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D13data=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=OD1YuhGY%2B4IZG7g8PMh%2Bk3amc6O9HjDV92pOFPd8RlE%3Dreserved=0
Signed-off-by: Sergii Romantsov 
Signed-off-by: Danylo Piliaiev 
Reviewed-by: Jason Ekstrand 

---

  src/mesa/drivers/dri/i965/brw_clear.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_clear.c 
b/src/mesa/drivers/dri/i965/brw_clear.c
index 30e09861491..1508171da10 100644
--- a/src/mesa/drivers/dri/i965/brw_clear.c
+++ b/src/mesa/drivers/dri/i965/brw_clear.c
@@ -167,7 +167,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
  */
 float clear_value =
mt->format == MESA_FORMAT_Z_FLOAT32 ? ctx->Depth.Clear :
-  (unsigned)(ctx->Depth.Clear * fb->_DepthMax) / (float)fb->_DepthMax;
+  _mesa_lroundeven(ctx->Depth.Clear * fb->_DepthMax) / 
(float)(fb->_DepthMax);
  
 const uint32_t num_layers = depth_att->Layered ? depth_irb->layer_count : 1;
  


___
mesa-commit mailing list
mesa-com...@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-commitdata=02%7C01%7Cbrianp%40vmware.com%7C6aa9bf59501d420b03af08d7175f0b0f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637003569293293608sdata=UWAkli3USsLDpnoElsuycRJ8KTrJV%2FC0r%2FarUay7SNQ%3Dreserved=0



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

Re: [Mesa-dev] [PATCH] gallivm: fix a missing argument to CreateAtomicCmpXchg

[Brian Paul]

On 08/01/2019 04:56 PM, Charmaine Lee wrote:

This patch fixes a missing argument to CreateAtomicCmpXchg for older
version of LLVM.
---
  src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp 
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 79d1029..8205d24 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -831,6 +831,7 @@ LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, 
LLVMValueRef Ptr,
 /* LLVM 3.8 doesn't have a second ordering and uses old 
SynchronizationScope enum */
 return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), 
llvm::unwrap(Cmp),
llvm::unwrap(New), 
mapFromLLVMOrdering(SuccessOrdering),
+  
mapFromLLVMOrdering(FailureOrdering),
SingleThread ? 
llvm::SynchronizationScope::SingleThread : 
llvm::SynchronizationScope::CrossThread));
  }
  #endif



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

Re: [Mesa-dev] [PATCH 1/2] nir/loop_analyze: used nir_alu_src to track loop limit

[Brian Paul]

On 06/19/2019 02:08 AM, Timothy Arceri wrote:

This helps reduce the amount of abstraction in this pass and allows
us to retain more information about the src such as any swizzles.
Retaining the swizzle information is required for a bugfix in the
following patch.

Fixes: 6772a17acc8e ("nir: Add a loop analysis pass")
---
  src/compiler/nir/nir_loop_analyze.c | 37 +++--
  1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index e85a404da1b..57d2d94cad2 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -543,25 +543,26 @@ guess_loop_limit(loop_info_state *state, nir_const_value 
*limit_val,
  }
  
  static bool

-try_find_limit_of_alu(nir_loop_variable *limit, nir_const_value *limit_val,
-  nir_loop_terminator *terminator, loop_info_state *state)
+try_find_limit_of_alu(nir_alu_src *limit, nir_const_value *limit_val,
+  nir_loop_terminator *terminator)
  {
-   if(!is_var_alu(limit))
+   if(limit->src.ssa->parent_instr->type != nir_instr_type_alu)
return false;
  
-   nir_alu_instr *limit_alu = nir_instr_as_alu(limit->def->parent_instr);

+   nir_alu_instr *limit_alu = nir_instr_as_alu(limit->src.ssa->parent_instr);
  
 if (limit_alu->op == nir_op_imin ||

 limit_alu->op == nir_op_fmin) {
-  limit = get_loop_var(limit_alu->src[0].src.ssa, state);
+  limit = _alu->src[0];
  
-  if (!is_var_constant(limit))

- limit = get_loop_var(limit_alu->src[1].src.ssa, state);
+  if (limit->src.ssa->parent_instr->type != nir_instr_type_load_const)
+ limit = _alu->src[1];
  
-  if (!is_var_constant(limit))

+  if (limit->src.ssa->parent_instr->type != nir_instr_type_load_const)
   return false;
  
-  *limit_val = nir_instr_as_load_const(limit->def->parent_instr)->value[0];

+  *limit_val =
+ nir_instr_as_load_const(limit->src.ssa->parent_instr)->value[0];
  
terminator->exact_trip_count_unknown = true;
  
@@ -777,19 +778,19 @@ is_supported_terminator_condition(nir_alu_instr *alu)
  
  static bool

  get_induction_and_limit_vars(nir_alu_instr *alu, nir_loop_variable **ind,
- nir_loop_variable **limit,
+ nir_alu_src **limit,
   loop_info_state *state)
  {
 bool limit_rhs = true;
  
 /* We assume that the limit is the "right" operand */

 *ind = get_loop_var(alu->src[0].src.ssa, state);
-   *limit = get_loop_var(alu->src[1].src.ssa, state);
+   *limit = >src[1];
  
 if ((*ind)->type != basic_induction) {

/* We had it the wrong way, flip things around */
*ind = get_loop_var(alu->src[1].src.ssa, state);
-  *limit = get_loop_var(alu->src[0].src.ssa, state);
+  *limit = >src[0];
limit_rhs = false;
 }
  
@@ -799,7 +800,7 @@ get_induction_and_limit_vars(nir_alu_instr *alu, nir_loop_variable **ind,

  static void
  try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
   nir_loop_variable **ind,
- nir_loop_variable **limit,
+ nir_alu_src **limit,
   bool *limit_rhs,
   loop_info_state *state)
  {
@@ -848,7 +849,7 @@ try_find_trip_count_vars_in_iand(nir_alu_instr **alu,
  
 /* Try the other iand src if needed */

 if (*ind == NULL || (*ind && (*ind)->type != basic_induction) ||
-   !is_var_constant(*limit)) {
+   (*limit)->src.ssa->parent_instr->type != nir_instr_type_load_const) {
src = iand->src[1].src.ssa;
if (src->parent_instr->type == nir_instr_type_alu) {
   nir_alu_instr *tmp_alu = nir_instr_as_alu(src->parent_instr);
@@ -891,7 +892,7 @@ find_trip_count(loop_info_state *state)
  
bool limit_rhs;

nir_loop_variable *basic_ind = NULL;
-  nir_loop_variable *limit;
+  nir_alu_src *limit;
if (alu->op == nir_op_inot || alu->op == nir_op_ieq) {
   nir_alu_instr *new_alu = alu;
   try_find_trip_count_vars_in_iand(_alu, _ind, ,
@@ -931,13 +932,13 @@ find_trip_count(loop_info_state *state)
  
/* Attempt to find a constant limit for the loop */

nir_const_value limit_val;
-  if (is_var_constant(limit)) {
+  if (limit->src.ssa->parent_instr->type == nir_instr_type_load_const) {
   limit_val =
-nir_instr_as_load_const(limit->def->parent_instr)->value[0];
+nir_instr_as_load_const(limit->src.ssa->parent_instr)->value[0];
} else {
   trip_count_known = false;
  
- if (!try_find_limit_of_alu(limit, _val, terminator, state)) {

+ if (!try_find_limit_of_alu(limit, _val, terminator)) {
  /* Guess loop limit based on array access */
  if (!guess_loop_limit(state, _val, 

Re: [Mesa-dev] [PATCH] nir: silence three compiler warnings seen with MinGW

[Brian Paul]

Ping.

-Brian

On 05/20/2019 07:36 AM, Brian Paul wrote:

Silence two unused var warnings.  And init elem_size, elem_align to
zero to silence "maybe uninitialized" warnings.
---
  src/compiler/nir/nir_lower_int_to_float.c | 2 +-
  src/compiler/nir/nir_opt_copy_prop_vars.c | 4 +---
  src/compiler/nir_types.cpp| 2 +-
  3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/compiler/nir/nir_lower_int_to_float.c 
b/src/compiler/nir/nir_lower_int_to_float.c
index 439afa0..66a740d9 100644
--- a/src/compiler/nir/nir_lower_int_to_float.c
+++ b/src/compiler/nir/nir_lower_int_to_float.c
@@ -28,7 +28,7 @@
  static bool
  assert_ssa_def_is_not_int(nir_ssa_def *def, void *arg)
  {
-   BITSET_WORD *int_types = arg;
+   MAYBE_UNUSED BITSET_WORD *int_types = arg;
 assert(!BITSET_TEST(int_types, def->index));
 return true;
  }
diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c 
b/src/compiler/nir/nir_opt_copy_prop_vars.c
index 94bc8af..0fd96b7 100644
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
@@ -433,9 +433,7 @@ load_element_from_ssa_entry_value(struct 
copy_prop_var_state *state,
nir_builder *b, nir_intrinsic_instr *intrin,
struct value *value, unsigned index)
  {
-   const struct glsl_type *type = entry->dst->type;
-   unsigned num_components = glsl_get_vector_elements(type);
-   assert(index < num_components);
+   assert(index < glsl_get_vector_elements(entry->dst->type));
  
 /* We don't have the element available, so let the instruction do the work. */

 if (!entry->src.ssa.def[index])
diff --git a/src/compiler/nir_types.cpp b/src/compiler/nir_types.cpp
index 3bf93c5..e2dfc40 100644
--- a/src/compiler/nir_types.cpp
+++ b/src/compiler/nir_types.cpp
@@ -630,7 +630,7 @@ glsl_get_natural_size_align_bytes(const struct glsl_type 
*type,
*size = 0;
*align = 0;
for (unsigned i = 0; i < type->length; i++) {
- unsigned elem_size, elem_align;
+ unsigned elem_size = 0, elem_align = 0;
   glsl_get_natural_size_align_bytes(type->fields.structure[i].type,
 _size, _align);
   *align = MAX2(*align, elem_align);



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

Re: [Mesa-dev] Mesa (master): glsl: do not use deprecated bison-keyword

[Brian Paul]

On 05/23/2019 02:48 AM, Erik Faye-Lund wrote:

On Wed, 2019-05-22 at 18:06 +0200, Akim Demaille wrote:

Hi Erik,


Le 22 mai 2019 à 08:54, Erik Faye-Lund <
erik.faye-l...@collabora.com> a écrit :

I ended up reverting the change [from-%error-verbose to %define
parse.error verbose] , and I can't find an obcious way to
support both old and new versions with the same source file.


Well, you just disable warnings about deprecated features.
-Wno-deprecated.


The Android SDK also ships a pre-3.x version of Bison, so you're
not
the only one affected. Also, Chocolatey doesn't provide a new
enough
version of Bison for Windows either.


I would be interested in knowing why some distros lag.  Do you
happen to know why for some of them?



The only clear examples I know of are the Android SDK and Chocolatey.
As I said before, we mighit not have to use the bison version shipped
in the Android SDK, so perhaps that's fixable. And for Chocolatey, it
turns out they have a winflexbison pacakge and a winflexbison3 package,
and the latter has a more recent Bison version.

So looking at this again, it looks like it might be solveable, at least
in the cases where this is visible to me.

Brian: do you think VMWare cuold upgrade the Bison version used in the
forseeable future?


I could start to work on that.  Getting new ancillary components into 
our build system is rather complex, but I've been through it a few times.


-Brian

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

Re: [Mesa-dev] Mesa (master): glsl: do not use deprecated bison-keyword

[Brian Paul]

I think this change broke the MSVC build for us.

I may not have time to investigate until later today.

-Brian


On 05/21/2019 05:41 AM, GitLab Mirror wrote:

Module: Mesa
Branch: master
Commit: eb85124a9f6e9cb94d0d4a99f91bbae374777e3a
URL:
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcgit.freedesktop.org%2Fmesa%2Fmesa%2Fcommit%2F%3Fid%3Deb85124a9f6e9cb94d0d4a99f91bbae374777e3adata=02%7C01%7Cbrianp%40vmware.com%7C8fb4b04aba0f46cf005d08d6dde15558%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636940357210597924sdata=N0sBC6rz%2F4KcpZyCKuNbNqNNhSE%2Fu6d8DcMCQY7STkY%3Dreserved=0

Author: Erik Faye-Lund 
Date:   Mon May 20 13:29:05 2019 +0200

glsl: do not use deprecated bison-keyword

%error-verbose has been deprecated since Bison 3.0, which was released
in 2013. In Bison 3.3.1 which was recently released, this has started
causing warnings. Let's update the code to do this in the modern way
intead, to avoid cluttering the output needlessly.

Signed-off-by: Erik Faye-Lund 
Reviewed-by: Timothy Arceri 

---

  src/compiler/glsl/glcpp/glcpp-parse.y | 2 +-
  src/compiler/glsl/glsl_parser.yy  | 2 +-
  src/mesa/program/program_parse.y  | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y 
b/src/compiler/glsl/glcpp/glcpp-parse.y
index 1c095cb66f9..736af7e680d 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -155,7 +155,7 @@ add_builtin_define(glcpp_parser_t *parser, const char 
*name, int value);
  %}
  
  %pure-parser

-%error-verbose
+%define parse.error verbose
  
  %locations

  %initial-action {
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index 6426f890b9e..dc6aade2643 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -81,7 +81,7 @@ static bool match_layout_qualifier(const char *s1, const char 
*s2,
  %expect 0
  
  %pure-parser

-%error-verbose
+%define parse.error verbose
  
  %locations

  %initial-action {
diff --git a/src/mesa/program/program_parse.y b/src/mesa/program/program_parse.y
index 7398f5f507a..3d0c1e2ea9e 100644
--- a/src/mesa/program/program_parse.y
+++ b/src/mesa/program/program_parse.y
@@ -124,7 +124,7 @@ static struct asm_instruction *asm_instruction_copy_ctor(
  %locations
  %lex-param   { struct asm_parser_state *state }
  %parse-param { struct asm_parser_state *state }
-%error-verbose
+%define parse.error verbose
  
  %union {

 struct asm_instruction *inst;

___
mesa-commit mailing list
mesa-com...@lists.freedesktop.org
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-commitdata=02%7C01%7Cbrianp%40vmware.com%7C8fb4b04aba0f46cf005d08d6dde15558%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636940357210607919sdata=%2F%2FK4CMY1Wd9PgydEgugYq63pp26NkX%2B4venu%2FYk7FQk%3Dreserved=0



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

Re: [Mesa-dev] [PATCH 2/2] mesa: unreference current winsys buffers when unbinding winsys buffers

[Brian Paul]

Both look OK to me.  Do they need to be tagged with "Cc: 
mesa-sta...@lists.freedesktop.org" for the stable branches?


Reviewed-by: Brian Paul 


On 05/18/2019 07:46 PM, Charmaine Lee wrote:

This fixes surface leak when no winsys buffers are bound.
---
  src/mesa/main/context.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 34da16b..04ef4d5 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1765,6 +1765,10 @@ _mesa_make_current( struct gl_context *newCtx,
  
   check_init_viewport(newCtx, drawBuffer->Width, drawBuffer->Height);

}
+  else {
+ _mesa_reference_framebuffer(>WinSysDrawBuffer, NULL);
+ _mesa_reference_framebuffer(>WinSysReadBuffer, NULL);
+  }
  
if (newCtx->FirstTimeCurrent) {

   handle_first_current(newCtx);



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

[Mesa-dev] [PATCH] nir: silence three compiler warnings seen with MinGW

[Brian Paul]

Silence two unused var warnings.  And init elem_size, elem_align to
zero to silence "maybe uninitialized" warnings.
---
 src/compiler/nir/nir_lower_int_to_float.c | 2 +-
 src/compiler/nir/nir_opt_copy_prop_vars.c | 4 +---
 src/compiler/nir_types.cpp| 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/compiler/nir/nir_lower_int_to_float.c 
b/src/compiler/nir/nir_lower_int_to_float.c
index 439afa0..66a740d9 100644
--- a/src/compiler/nir/nir_lower_int_to_float.c
+++ b/src/compiler/nir/nir_lower_int_to_float.c
@@ -28,7 +28,7 @@
 static bool
 assert_ssa_def_is_not_int(nir_ssa_def *def, void *arg)
 {
-   BITSET_WORD *int_types = arg;
+   MAYBE_UNUSED BITSET_WORD *int_types = arg;
assert(!BITSET_TEST(int_types, def->index));
return true;
 }
diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c 
b/src/compiler/nir/nir_opt_copy_prop_vars.c
index 94bc8af..0fd96b7 100644
--- a/src/compiler/nir/nir_opt_copy_prop_vars.c
+++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
@@ -433,9 +433,7 @@ load_element_from_ssa_entry_value(struct 
copy_prop_var_state *state,
   nir_builder *b, nir_intrinsic_instr *intrin,
   struct value *value, unsigned index)
 {
-   const struct glsl_type *type = entry->dst->type;
-   unsigned num_components = glsl_get_vector_elements(type);
-   assert(index < num_components);
+   assert(index < glsl_get_vector_elements(entry->dst->type));
 
/* We don't have the element available, so let the instruction do the work. 
*/
if (!entry->src.ssa.def[index])
diff --git a/src/compiler/nir_types.cpp b/src/compiler/nir_types.cpp
index 3bf93c5..e2dfc40 100644
--- a/src/compiler/nir_types.cpp
+++ b/src/compiler/nir_types.cpp
@@ -630,7 +630,7 @@ glsl_get_natural_size_align_bytes(const struct glsl_type 
*type,
   *size = 0;
   *align = 0;
   for (unsigned i = 0; i < type->length; i++) {
- unsigned elem_size, elem_align;
+ unsigned elem_size = 0, elem_align = 0;
  glsl_get_natural_size_align_bytes(type->fields.structure[i].type,
_size, _align);
  *align = MAX2(*align, elem_align);
-- 
2.7.4

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

[Mesa-dev] [PATCH] svga: clamp max_const_buffers to SVGA_MAX_CONST_BUFS

[Brian Paul]

In case the device reports 15 (or more) buffers.
---
 src/gallium/drivers/svga/svga_screen.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/svga/svga_screen.c 
b/src/gallium/drivers/svga/svga_screen.c
index 02c1a99..b70fd85 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -1079,7 +1079,8 @@ svga_screen_create(struct svga_winsys_screen *sws)
   /* Maximum number of constant buffers */
   svgascreen->max_const_buffers =
  get_uint_cap(sws, SVGA3D_DEVCAP_DX_MAX_CONSTANT_BUFFERS, 1);
-  assert(svgascreen->max_const_buffers <= SVGA_MAX_CONST_BUFS);
+  svgascreen->max_const_buffers = MIN2(svgascreen->max_const_buffers,
+   SVGA_MAX_CONST_BUFS);
 
   screen->is_format_supported = svga_is_dx_format_supported;
}
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 0/6] Update mesa state handling past VAO changes.

[Brian Paul]

The series LGTM.

Reviewed-by: Brian Paul 


On 05/12/2019 07:05 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

Hi Brian,

The series is a collection of comment updates and
state handling cleanup past the VAO changes that went
into mesa. There are two fixes for potential bugs
in state handling included. There were no reports accoring
that, and it does not change the outcome of intels CI.

please review

best

Mathias


Mathias Fröhlich (6):
   mesa/vbo: Update Comment to what is actually happening.
   mesa: Fix old outdated variable name in a comment.
   mesa: Make _mesa_set_varying_vp_inputs static in state.c.
   mesa: Fix test for setting the _NEW_VARYING_VP_INPUTS flag.
   mesa: Avoid setting _NEW_VARYING_VP_INPUTS in non fixed function mode.
   mesa: Set _NEW_VARYING_VP_INPUTS iff varying_vp_inputs are set.

  src/mesa/main/ff_fragment_shader.cpp |  2 ++
  src/mesa/main/ffvertex_prog.c|  3 +++
  src/mesa/main/state.c| 38 +++-
  src/mesa/main/state.h|  6 +
  src/mesa/vbo/vbo_exec_draw.c |  4 +--
  5 files changed, 27 insertions(+), 26 deletions(-)

--
2.21.0



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

[Mesa-dev] [PATCH] st/mesa: fix uninitialized lower_flrp_progress variable

[Brian Paul]

The 'progress' variable is initialized to false in other locations.
This fixes a new Coverity warning.
---
 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 0a67d45..5706425 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -338,7 +338,7 @@ st_nir_opts(nir_shader *nir, bool scalar)
   NIR_PASS(progress, nir, nir_opt_constant_folding);
 
   if (lower_flrp != 0) {
- bool lower_flrp_progress;
+ bool lower_flrp_progress = false;
 
  NIR_PASS(lower_flrp_progress, nir, nir_lower_flrp,
   lower_flrp,
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] gallivm: fix broken 8-wide s3tc decoding

[Brian Paul]

LGTM.  Just two little nits below.

Reviewed-by: Brian Paul 

Perhaps you could review the 5-patch series of clean-ups I posted on 
Saturday?



On 05/06/2019 06:12 PM, srol...@vmware.com wrote:

From: Roland Scheidegger 

Brian noticed there was an uninitialized var for the 8-wide case and 128
bit blocks, which made it always crash. Likewise, the 64bit block case
had another crash bug due to type mismatch.
Color decode (used for all s3tc formats) also had a bogus shuffle for
this case, leading to decode artifacts.
Fix these all up, which makes the code actually work 8-wide. Note that
it's still not used - I've verified it works, and the generated assembly
does look quite a bit simpler actually (20-30% less instructions for the
s3tc decode part with avx2), however in practice it still seems to be
sligthly slower for some unknown reason (tested with openarena) on my
haswell box, so for now continue to split things into 4-wide vectors
before decoding.
---
  .../auxiliary/gallivm/lp_bld_format_s3tc.c| 33 +--
  1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c 
b/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
index 9561c349dad..8f6e9bec18a 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
@@ -77,24 +77,17 @@ lp_build_uninterleave2_half(struct gallivm_state *gallivm,
  unsigned lo_hi)
  {
 LLVMValueRef shuffle, elems[LP_MAX_VECTOR_LENGTH];
-   unsigned i, j;
+   unsigned i;
  
 assert(type.length <= LP_MAX_VECTOR_LENGTH);

 assert(lo_hi < 2);
  
 if (type.length * type.width == 256) {

-  assert(type.length >= 4);
-  for (i = 0, j = 0; i < type.length; ++i) {
- if (i == type.length / 4) {
-j = type.length;
- } else if (i == type.length / 2) {
-j = type.length / 2;
- } else if (i == 3 * type.length / 4) {
-j = 3 * type.length / 4;
- } else {
-j += 2;
- }
- elems[i] = lp_build_const_int32(gallivm, j + lo_hi);
+  assert(type.length == 8);
+  assert(type.width == 32);
+  const unsigned shufvals[8] = {0, 2, 8, 10, 4, 6, 12, 14};


could be static



+  for (i = 0; i < type.length; ++i) {
+ elems[i] = lp_build_const_int32(gallivm, shufvals[i] + lo_hi);
}
 } else {
for (i = 0; i < type.length; ++i) {
@@ -277,7 +270,7 @@ lp_build_gather_s3tc(struct gallivm_state *gallivm,
 }
 else {
LLVMValueRef tmp[4], cc01, cc23;
-  struct lp_type lp_type32, lp_type64, lp_type32dxt;
+  struct lp_type lp_type32, lp_type64;
memset(_type32, 0, sizeof lp_type32);
lp_type32.width = 32;
lp_type32.length = length;
@@ -309,10 +302,14 @@ lp_build_gather_s3tc(struct gallivm_state *gallivm,
lp_build_const_extend_shuffle(gallivm, 2, 
4), "");
   }
   if (length == 8) {
+struct lp_type lp_type32_4;
+memset(_type32_4, 0, sizeof lp_type32_4);


Maybe we could move toward using initializers in these case?  struct 
lp_type lp_type32_4 = {0}; ?




+lp_type32_4.width = 32;
+lp_type32_4.length = 4;
  for (i = 0; i < 4; ++i) {
 tmp[0] = elems[i];
 tmp[1] = elems[i+4];
-   elems[i] = lp_build_concat(gallivm, tmp, lp_type32, 2);
+   elems[i] = lp_build_concat(gallivm, tmp, lp_type32_4, 2);
  }
   }
   cc01 = lp_build_interleave2_half(gallivm, lp_type32, elems[0], 
elems[1], 0);
@@ -811,7 +808,7 @@ s3tc_dxt3_to_rgba_aos(struct gallivm_state *gallivm,
 tmp = lp_build_select(, sel_mask, alpha_low, alpha_hi);
 bit_pos = LLVMBuildAnd(builder, bit_pos,
lp_build_const_int_vec(gallivm, type, 0xffdf), 
"");
-   /* Warning: slow shift with per element count */
+   /* Warning: slow shift with per element count (without avx2) */
 /*
  * Could do pshufb here as well - just use appropriate 2 bits in bit_pos
  * to select the right byte with pshufb. Then for the remaining one bit
@@ -1640,7 +1637,6 @@ s3tc_decode_block_dxt5(struct gallivm_state *gallivm,
lp_build_const_int_vec(gallivm, type16, 8), "");
 alpha = LLVMBuildBitCast(builder, alpha,  i64t, "");
 shuffle1 = lp_build_const_shuffle1(gallivm, 0, 8);
-   /* XXX this shuffle broken with LLVM 2.8 */
 alpha0 = LLVMBuildShuffleVector(builder, alpha0, alpha0, shuffle1, "");
 alpha1 = LLVMBuildShuffleVector(builder, alpha1, alpha1, shuffle1, "");
  
@@ -2176,6 +2172,9 @@ lp_build_fetch_s3tc_rgba_aos(struct gallivm_state *gallivm,

return rgba;
 }
  
+   /*

+* Could use n > 8 here with avx2, but doesn't seem fast

[Mesa-dev] [PATCH 2/5] ddebug: fix a few MSVC compiler warnings

[Brian Paul]

Don't return an expression in void functions.
Replace an unsigned int with proper enum.
---
 src/gallium/auxiliary/driver_ddebug/dd_context.c | 15 ---
 src/gallium/auxiliary/driver_ddebug/dd_screen.c  |  2 +-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/gallium/auxiliary/driver_ddebug/dd_context.c 
b/src/gallium/auxiliary/driver_ddebug/dd_context.c
index 8a3b75a..001a69f 100644
--- a/src/gallium/auxiliary/driver_ddebug/dd_context.c
+++ b/src/gallium/auxiliary/driver_ddebug/dd_context.c
@@ -534,7 +534,8 @@ dd_context_set_shader_images(struct pipe_context *_pipe,
 }
 
 static void
-dd_context_set_shader_buffers(struct pipe_context *_pipe, unsigned shader,
+dd_context_set_shader_buffers(struct pipe_context *_pipe,
+  enum pipe_shader_type shader,
   unsigned start, unsigned num_buffers,
   const struct pipe_shader_buffer *buffers,
   unsigned writable_bitmask)
@@ -680,7 +681,7 @@ dd_context_set_compute_resources(struct pipe_context *_pipe,
 struct pipe_surface **resources)
 {
struct pipe_context *pipe = dd_context(_pipe)->pipe;
-   return pipe->set_compute_resources(pipe, start, count, resources);
+   pipe->set_compute_resources(pipe, start, count, resources);
 }
 
 static void
@@ -690,7 +691,7 @@ dd_context_set_global_binding(struct pipe_context *_pipe,
  uint32_t **handles)
 {
struct pipe_context *pipe = dd_context(_pipe)->pipe;
-   return pipe->set_global_binding(pipe, first, count, resources, handles);
+   pipe->set_global_binding(pipe, first, count, resources, handles);
 }
 
 static void
@@ -700,8 +701,8 @@ dd_context_get_sample_position(struct pipe_context *_pipe,
 {
struct pipe_context *pipe = dd_context(_pipe)->pipe;
 
-   return pipe->get_sample_position(pipe, sample_count, sample_index,
-out_value);
+   pipe->get_sample_position(pipe, sample_count, sample_index,
+ out_value);
 }
 
 static void
@@ -727,7 +728,7 @@ dd_context_set_device_reset_callback(struct pipe_context 
*_pipe,
 {
struct pipe_context *pipe = dd_context(_pipe)->pipe;
 
-   return pipe->set_device_reset_callback(pipe, cb);
+   pipe->set_device_reset_callback(pipe, cb);
 }
 
 static void
@@ -747,7 +748,7 @@ dd_context_dump_debug_state(struct pipe_context *_pipe, 
FILE *stream,
 {
struct pipe_context *pipe = dd_context(_pipe)->pipe;
 
-   return pipe->dump_debug_state(pipe, stream, flags);
+   pipe->dump_debug_state(pipe, stream, flags);
 }
 
 static uint64_t
diff --git a/src/gallium/auxiliary/driver_ddebug/dd_screen.c 
b/src/gallium/auxiliary/driver_ddebug/dd_screen.c
index ce9f697..f3bd079 100644
--- a/src/gallium/auxiliary/driver_ddebug/dd_screen.c
+++ b/src/gallium/auxiliary/driver_ddebug/dd_screen.c
@@ -126,7 +126,7 @@ static void dd_screen_query_memory_info(struct pipe_screen 
*_screen,
 {
struct pipe_screen *screen = dd_screen(_screen)->screen;
 
-   return screen->query_memory_info(screen, info);
+   screen->query_memory_info(screen, info);
 }
 
 static struct pipe_context *
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 1/5] glsl: s/GLboolean/bool/ to silence MSVC compiler warning

[Brian Paul]

It complains about mixing GLboolean and bool in the |= expression.
---
 src/compiler/glsl/glsl_parser_extras.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index d99ab3d..41f2a97 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -2226,7 +2226,7 @@ do_common_optimization(exec_list *ir, bool linked,
bool native_integers)
 {
const bool debug = false;
-   GLboolean progress = GL_FALSE;
+   bool progress = false;
 
 #define OPT(PASS, ...) do { \
   if (debug) {  \
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 4/5] gallium/pp: s/uint/enum tgsi_semantic/ to fix MSVC warning

[Brian Paul]

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

diff --git a/src/gallium/auxiliary/postprocess/pp_program.c 
b/src/gallium/auxiliary/postprocess/pp_program.c
index 52786de..4cd3990 100644
--- a/src/gallium/auxiliary/postprocess/pp_program.c
+++ b/src/gallium/auxiliary/postprocess/pp_program.c
@@ -126,7 +126,7 @@ pp_init_prog(struct pp_queue_t *ppq, struct pipe_context 
*pipe,
 
 
{
-  const uint semantic_names[] = { TGSI_SEMANTIC_POSITION,
+  const enum tgsi_semantic semantic_names[] = { TGSI_SEMANTIC_POSITION,
  TGSI_SEMANTIC_GENERIC
   };
   const uint semantic_indexes[] = { 0, 0 };
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 3/5] noop: s/enum pipe_transfer_usage/unsigned/ to fix MSVC warning

[Brian Paul]

The function pointer declaration in pipe_context uses unsigned
for the bitmask.
---
 src/gallium/auxiliary/driver_noop/noop_pipe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/driver_noop/noop_pipe.c 
b/src/gallium/auxiliary/driver_noop/noop_pipe.c
index a6497f0..2a4d3eb 100644
--- a/src/gallium/auxiliary/driver_noop/noop_pipe.c
+++ b/src/gallium/auxiliary/driver_noop/noop_pipe.c
@@ -172,7 +172,7 @@ static void noop_resource_destroy(struct pipe_screen 
*screen,
 static void *noop_transfer_map(struct pipe_context *pipe,
struct pipe_resource *resource,
unsigned level,
-   enum pipe_transfer_usage usage,
+   unsigned usage,
const struct pipe_box *box,
struct pipe_transfer **ptransfer)
 {
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 5/5] gallium/util: fix two MSVC compiler warnings

[Brian Paul]

Remove stray const qualifier.
s/unsigned/enum tgsi_semantic/
---
 src/gallium/auxiliary/util/u_format_zs.h  | 2 +-
 src/gallium/auxiliary/util/u_simple_shaders.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_format_zs.h 
b/src/gallium/auxiliary/util/u_format_zs.h
index 160919d..bed3c51 100644
--- a/src/gallium/auxiliary/util/u_format_zs.h
+++ b/src/gallium/auxiliary/util/u_format_zs.h
@@ -113,7 +113,7 @@ void
 util_format_z24_unorm_s8_uint_pack_s_8uint(uint8_t *dst_row, unsigned 
dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, 
unsigned height);
 
 void
-util_format_z24_unorm_s8_uint_pack_separate(uint8_t *dst_row, unsigned 
dst_stride, const uint32_t *z_src_row, unsigned z_src_stride, const uint8_t 
*s_src_row, unsigned s_src_stride, const unsigned width, unsigned height);
+util_format_z24_unorm_s8_uint_pack_separate(uint8_t *dst_row, unsigned 
dst_stride, const uint32_t *z_src_row, unsigned z_src_stride, const uint8_t 
*s_src_row, unsigned s_src_stride, unsigned width, unsigned height);
 
 void
 util_format_s8_uint_z24_unorm_unpack_z_float(float *dst_row, unsigned 
dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, 
unsigned height);
diff --git a/src/gallium/auxiliary/util/u_simple_shaders.c 
b/src/gallium/auxiliary/util/u_simple_shaders.c
index 4046ab1..d62a655 100644
--- a/src/gallium/auxiliary/util/u_simple_shaders.c
+++ b/src/gallium/auxiliary/util/u_simple_shaders.c
@@ -117,8 +117,8 @@ util_make_vertex_passthrough_shader_with_so(struct 
pipe_context *pipe,
 
 void *util_make_layered_clear_vertex_shader(struct pipe_context *pipe)
 {
-   const unsigned semantic_names[] = {TGSI_SEMANTIC_POSITION,
-  TGSI_SEMANTIC_GENERIC};
+   const enum tgsi_semantic semantic_names[] = {TGSI_SEMANTIC_POSITION,
+TGSI_SEMANTIC_GENERIC};
const unsigned semantic_indices[] = {0, 0};
 
return util_make_vertex_passthrough_shader_with_so(pipe, 2, semantic_names,
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] nir/algebraic: Don't emit empty arrays for MSVC

[Brian Paul]

Just don't emit the transform array at all if there are no transforms
for a state, and avoid trying to walk over it.

Original patch by Connor Abbott.  Updated with suggestions by
Dylan Baker.
---
 src/compiler/nir/nir_algebraic.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 6db749e..da5e39c 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -993,11 +993,13 @@ static const uint16_t CONST_STATE = 1;
 % endfor
 
 % for state_id, state_xforms in enumerate(automaton.state_patterns):
+% if state_xforms:  # avoid emitting a 0-length array for MSVC
 static const struct transform ${pass_name}_state${state_id}_xforms[] = {
 % for i in state_xforms:
   { ${xforms[i].search.c_ptr(cache)}, ${xforms[i].replace.c_value_ptr(cache)}, 
${xforms[i].condition_index} },
 % endfor
 };
+% endif
 % endfor
 
 static const struct per_op_table ${pass_name}_table[nir_num_search_ops] = {
@@ -1080,7 +1082,8 @@ ${pass_name}_block(nir_builder *build, nir_block *block,
   switch (states[alu->dest.dest.ssa.index]) {
 % for i in range(len(automaton.state_patterns)):
   case ${i}:
- for (unsigned i = 0; i < ARRAY_SIZE(${pass_name}_state${i}_xforms); 
i++) {
+ % if automaton.state_patterns[i]:
+ for (unsigned i = 0; i < ${len(automaton.state_patterns[i])}; i++) {
 const struct transform *xform = &${pass_name}_state${i}_xforms[i];
 if (condition_flags[xform->condition_offset] &&
 nir_replace_instr(build, alu, xform->search, xform->replace)) {
@@ -1088,6 +1091,7 @@ ${pass_name}_block(nir_builder *build, nir_block *block,
break;
 }
  }
+ % endif
  break;
 % endfor
   default: assert(0);
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

[Brian Paul]

On 05/02/2019 11:18 PM, Mathias Fröhlich wrote:

Hi Brian,

On Friday, 3 May 2019 00:17:51 CEST Brian Paul wrote:

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

In glArrayElement, use the bitmask trick to just walk the enabled
vao arrays. This should be about equivalent in execution time to
walk the prepare aelt_context list. Finally this will allow us to
reduce the _mesa_update_state calls in a few patches.

Signed-off-by: Mathias Fröhlich 
---
   src/mesa/main/api_arrayelt.c | 78 
   1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index d46c8d14b68..62f1e73ca4c 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
   }


+static inline attrib_func
+func_nv(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline attrib_func
+func_arb(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline const void *
+attrib_src(const struct gl_vertex_array_object *vao,
+   const struct gl_array_attributes *array, GLint elt)
+{
+   const struct gl_vertex_buffer_binding *binding =
+  >BufferBinding[array->BufferBindingIndex];
+   const GLubyte *src
+  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
+ _mesa_vertex_attrib_address(array, binding))
+  + elt * binding->Stride;
+   return src;
+}


Could you add some brief comments on those functions to explain what
they do?


Added brief comments:

/*
  * Return VertexAttrib*NV function pointer matching the provided vertex format.
  */
static inline attrib_func
func_nv(const struct gl_vertex_format *vformat)

[...]

/*
  * Return VertexAttrib*ARB function pointer matching the provided vertex 
format.
  */
static inline attrib_func
func_arb(const struct gl_vertex_format *vformat)

[...]

/*
  * Return the address of the array attribute array at elt in the
  * vertex array object vao.
  */
static inline const void *
attrib_src(const struct gl_vertex_array_object *vao,
const struct gl_array_attributes *array, GLint elt)



Otherwise, for the rest of the series,
Reviewed-by: Brian Paul 

Nice work!!


Thanks for looking at the patches!


All your suggested changes look good.

Reviewed-by: Brian Paul 

Thanks.

-Brian



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

Re: [Mesa-dev] [PATCH 03/10] mesa: Implement _mesa_array_element by walking enabled arrays.

[Brian Paul]

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

In glArrayElement, use the bitmask trick to just walk the enabled
vao arrays. This should be about equivalent in execution time to
walk the prepare aelt_context list. Finally this will allow us to
reduce the _mesa_update_state calls in a few patches.

Signed-off-by: Mathias Fröhlich 
---
  src/mesa/main/api_arrayelt.c | 78 
  1 file changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index d46c8d14b68..62f1e73ca4c 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -1541,32 +1541,76 @@ _ae_update_state(struct gl_context *ctx)
  }


+static inline attrib_func
+func_nv(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsNV[vformat->Normalized][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline attrib_func
+func_arb(const struct gl_vertex_format *vformat)
+{
+   return AttribFuncsARB[NORM_IDX(vformat)][vformat->Size-1]
+  [TYPE_IDX(vformat->Type)];
+}
+
+
+static inline const void *
+attrib_src(const struct gl_vertex_array_object *vao,
+   const struct gl_array_attributes *array, GLint elt)
+{
+   const struct gl_vertex_buffer_binding *binding =
+  >BufferBinding[array->BufferBindingIndex];
+   const GLubyte *src
+  = ADD_POINTERS(binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
+ _mesa_vertex_attrib_address(array, binding))
+  + elt * binding->Stride;
+   return src;
+}


Could you add some brief comments on those functions to explain what 
they do?


Otherwise, for the rest of the series,
Reviewed-by: Brian Paul 

Nice work!!

-Brian



+
+
  void
  _mesa_array_element(struct gl_context *ctx,
  struct _glapi_table *disp, GLint elt)
  {
-   const AEcontext *actx = AE_CONTEXT(ctx);
+   const struct gl_vertex_array_object *vao = ctx->Array.VAO;
+   GLbitfield mask;

-   if (actx->dirty_state)
-  _ae_update_state(ctx);
+   /* emit conventional arrays elements */
+   mask = (VERT_BIT_FF_ALL & ~VERT_BIT_POS) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(attrib, src);
+   }

 /* emit generic attribute elements */
-   for (const AEattrib *at = actx->attribs; at->func; at++) {
-  const GLubyte *src
- = ADD_POINTERS(at->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(at->array, at->binding))
- + elt * at->binding->Stride;
-  at->func(at->index, src);
+   mask = (VERT_BIT_GENERIC_ALL & ~VERT_BIT_GENERIC0) & vao->Enabled;
+   while (mask) {
+  const gl_vert_attrib attrib = u_bit_scan();
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_arb(>Format)(attrib - VERT_ATTRIB_GENERIC0, src);
 }

-   /* emit conventional arrays elements */
-   for (const AEarray *aa = actx->arrays; aa->offset != -1 ; aa++) {
-  const GLubyte *src
- = ADD_POINTERS(aa->binding->BufferObj->Mappings[MAP_INTERNAL].Pointer,
-_mesa_vertex_attrib_address(aa->array, aa->binding))
- + elt * aa->binding->Stride;
-  CALL_by_offset(disp, (array_func), aa->offset, ((const void *) src));
-   }
+   /* finally, vertex position */
+   if (vao->Enabled & VERT_BIT_GENERIC0) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_GENERIC0;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  /* Use glVertex(v) instead of glVertexAttrib(0, v) to be sure it's
+   * issued as the last (provoking) attribute).
+   */
+  func_nv(>Format)(0, src);
+   } else if (vao->Enabled & VERT_BIT_POS) {
+  const gl_vert_attrib attrib = VERT_ATTRIB_POS;
+  const struct gl_array_attributes *array = >VertexAttrib[attrib];
+  const void *src = attrib_src(vao, array, elt);
+  func_nv(>Format)(0, src);
+}
  }


--
2.20.1



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

Re: [Mesa-dev] [PATCH 01/10] mesa: Factor out index function that will have multiple use.

[Brian Paul]

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

For access to glArrayElement methods factor out a function to
get the table lookup index for normalized/integer/double access.
The function will be used in the next patch at least twice.

Signed-off-by: Mathias Fröhlich 
---
  src/mesa/main/api_arrayelt.c | 29 ++---
  1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index 1c086bbcda4..3f16e18b44d 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -90,6 +90,23 @@ TYPE_IDX(GLenum t)
  }


+/*
+ * Convert normalized/integer/double to the range [0, 3].
+ */
+static inline int
+NORM_IDX(const struct gl_vertex_format *vformat)


Maybe we could find a better name.  How about vertex_format_to_index()?

-Brian



+{
+   if (vformat->Doubles)
+  return 3;
+   else if (vformat->Integer)
+  return 2;
+   else if (vformat->Normalized)
+  return 1;
+   else
+  return 0;
+}
+
+
  bool
  _ae_is_state_dirty(struct gl_context *ctx)
  {
@@ -1610,7 +1627,6 @@ _ae_update_state(struct gl_context *ctx)
if (vao->Enabled & VERT_BIT_GENERIC(i)) {
   struct gl_array_attributes *attribArray =
  >VertexAttrib[VERT_ATTRIB_GENERIC(i)];
- GLint intOrNorm;
   at->array = attribArray;
   at->binding = >BufferBinding[attribArray->BufferBindingIndex];
   /* Note: we can't grab the _glapi_Dispatch->VertexAttrib1fvNV
@@ -1618,16 +1634,7 @@ _ae_update_state(struct gl_context *ctx)
* change from one execution of _ae_ArrayElement() to
* the next.  Doing so caused UT to break.
*/
- if (at->array->Format.Doubles)
-intOrNorm = 3;
- else if (at->array->Format.Integer)
-intOrNorm = 2;
- else if (at->array->Format.Normalized)
-intOrNorm = 1;
- else
-intOrNorm = 0;
-
- at->func = AttribFuncsARB[intOrNorm]
+ at->func = AttribFuncsARB[NORM_IDX(>array->Format)]
  [at->array->Format.Size-1]
  [TYPE_IDX(at->array->Format.Type)];

--
2.20.1



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

Re: [Mesa-dev] [PATCH 02/10] mesa: Use glVertexAttrib*NV functions for fixed function attribs.

[Brian Paul]

On 05/02/2019 03:27 AM, mathias.froehl...@gmx.net wrote:

From: Mathias Fröhlich 

In the glArrayElement implementation, use glVertexAttrib*NV type
functions for fixed function attributes. We do the same in display
execution when the list is replayed using immediate mode attribute
functions. By that use a loop to walk the attributes.


I'm not sure I understand that last sentence.

The code looks fine.

Reviewed-by: Brian Paul 




Signed-off-by: Mathias Fröhlich 
---
  src/mesa/main/api_arrayelt.c | 185 ++-
  1 file changed, 28 insertions(+), 157 deletions(-)

diff --git a/src/mesa/main/api_arrayelt.c b/src/mesa/main/api_arrayelt.c
index 3f16e18b44d..d46c8d14b68 100644
--- a/src/mesa/main/api_arrayelt.c
+++ b/src/mesa/main/api_arrayelt.c
@@ -117,89 +117,6 @@ _ae_is_state_dirty(struct gl_context *ctx)
  #define NUM_TYPES 8


-static const int ColorFuncs[2][NUM_TYPES] = {
-   {
-  _gloffset_Color3bv,
-  _gloffset_Color3ubv,
-  _gloffset_Color3sv,
-  _gloffset_Color3usv,
-  _gloffset_Color3iv,
-  _gloffset_Color3uiv,
-  _gloffset_Color3fv,
-  _gloffset_Color3dv,
-   },
-   {
-  _gloffset_Color4bv,
-  _gloffset_Color4ubv,
-  _gloffset_Color4sv,
-  _gloffset_Color4usv,
-  _gloffset_Color4iv,
-  _gloffset_Color4uiv,
-  _gloffset_Color4fv,
-  _gloffset_Color4dv,
-   },
-};
-
-static const int VertexFuncs[3][NUM_TYPES] = {
-   {
-  -1,
-  -1,
-  _gloffset_Vertex2sv,
-  -1,
-  _gloffset_Vertex2iv,
-  -1,
-  _gloffset_Vertex2fv,
-  _gloffset_Vertex2dv,
-   },
-   {
-  -1,
-  -1,
-  _gloffset_Vertex3sv,
-  -1,
-  _gloffset_Vertex3iv,
-  -1,
-  _gloffset_Vertex3fv,
-  _gloffset_Vertex3dv,
-   },
-   {
-  -1,
-  -1,
-  _gloffset_Vertex4sv,
-  -1,
-  _gloffset_Vertex4iv,
-  -1,
-  _gloffset_Vertex4fv,
-  _gloffset_Vertex4dv,
-   },
-};
-
-static const int IndexFuncs[NUM_TYPES] = {
-   -1,
-   _gloffset_Indexubv,
-   _gloffset_Indexsv,
-   -1,
-   _gloffset_Indexiv,
-   -1,
-   _gloffset_Indexfv,
-   _gloffset_Indexdv,
-};
-
-static const int NormalFuncs[NUM_TYPES] = {
-   _gloffset_Normal3bv,
-   -1,
-   _gloffset_Normal3sv,
-   -1,
-   _gloffset_Normal3iv,
-   -1,
-   _gloffset_Normal3fv,
-   _gloffset_Normal3dv,
-};
-
-/* Note: _gloffset_* for these may not be a compile-time constant. */
-static int SecondaryColorFuncs[NUM_TYPES];
-static int FogCoordFuncs[NUM_TYPES];
-
-
  /**
   ** GL_NV_vertex_program
   **/
@@ -1508,25 +1425,6 @@ _ae_create_context(struct gl_context *ctx)
 if (ctx->aelt_context)
return GL_TRUE;

-   /* These _gloffset_* values may not be compile-time constants */
-   SecondaryColorFuncs[0] = _gloffset_SecondaryColor3bv;
-   SecondaryColorFuncs[1] = _gloffset_SecondaryColor3ubv;
-   SecondaryColorFuncs[2] = _gloffset_SecondaryColor3sv;
-   SecondaryColorFuncs[3] = _gloffset_SecondaryColor3usv;
-   SecondaryColorFuncs[4] = _gloffset_SecondaryColor3iv;
-   SecondaryColorFuncs[5] = _gloffset_SecondaryColor3uiv;
-   SecondaryColorFuncs[6] = _gloffset_SecondaryColor3fvEXT;
-   SecondaryColorFuncs[7] = _gloffset_SecondaryColor3dv;
-
-   FogCoordFuncs[0] = -1;
-   FogCoordFuncs[1] = -1;
-   FogCoordFuncs[2] = -1;
-   FogCoordFuncs[3] = -1;
-   FogCoordFuncs[4] = -1;
-   FogCoordFuncs[5] = -1;
-   FogCoordFuncs[6] = _gloffset_FogCoordfvEXT;
-   FogCoordFuncs[7] = _gloffset_FogCoorddv;
-
 ctx->aelt_context = calloc(1, sizeof(AEcontext));
 if (!ctx->aelt_context)
return GL_FALSE;
@@ -1562,52 +1460,10 @@ _ae_update_state(struct gl_context *ctx)
 struct gl_vertex_array_object *vao = ctx->Array.VAO;

 /* conventional vertex arrays */
-   if (vao->Enabled & VERT_BIT_COLOR_INDEX) {
-  aa->array = >VertexAttrib[VERT_ATTRIB_COLOR_INDEX];
-  aa->binding = >BufferBinding[aa->array->BufferBindingIndex];
-  aa->offset = IndexFuncs[TYPE_IDX(aa->array->Format.Type)];
-  aa++;
-   }
-
-   if (vao->Enabled & VERT_BIT_EDGEFLAG) {
-  aa->array = >VertexAttrib[VERT_ATTRIB_EDGEFLAG];
-  aa->binding = >BufferBinding[aa->array->BufferBindingIndex];
-  aa->offset = _gloffset_EdgeFlagv;
-  aa++;
-   }
-
-   if (vao->Enabled & VERT_BIT_NORMAL) {
-  aa->array = >VertexAttrib[VERT_ATTRIB_NORMAL];
-  aa->binding = >BufferBinding[aa->array->BufferBindingIndex];
-  aa->offset = NormalFuncs[TYPE_IDX(aa->array->Format.Type)];
-  aa++;
-   }
-
-   if (vao->Enabled & VERT_BIT_COLOR0) {
-  aa->array = >VertexAttrib[VERT_ATTRIB_COLOR0];
-  aa->binding = >BufferBinding[aa->array->BufferBindingIndex];
-  aa->offset = 
ColorFuncs[aa->array->Format.Size-3][TYPE_IDX(aa->array->Format.Type)];
-  aa++;
-   }
-
-   if (vao->Enabled & VERT_BIT_COLOR1) {
-  aa->array = >VertexAttrib[VERT_ATTRIB_

Re: [Mesa-dev] [PATCH] nir/algebraic: Don't emit empty initializers for MSVC

[Brian Paul]

On 05/02/2019 02:34 PM, Connor Abbott wrote:

Just don't emit the transform array at all if there are no transforms
for a state, and avoid trying to walk over it.
---
Brian, does this build on Windows? I tested it on my shader-db
on radeonsi.


Yes, it compiles.  Thanks!

Tested-by: Brian Paul 

Though it looks like Dylan had some suggestions.

-Brian



---
  src/compiler/nir/nir_algebraic.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 6db749e9248..7af80a4f92e 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -993,11 +993,13 @@ static const uint16_t CONST_STATE = 1;
  % endfor
  
  % for state_id, state_xforms in enumerate(automaton.state_patterns):

+% if len(state_xforms) > 0: # avoid emitting a 0-length array for MSVC
  static const struct transform ${pass_name}_state${state_id}_xforms[] = {
  % for i in state_xforms:
{ ${xforms[i].search.c_ptr(cache)}, 
${xforms[i].replace.c_value_ptr(cache)}, ${xforms[i].condition_index} },
  % endfor
  };
+% endif
  % endfor
  
  static const struct per_op_table ${pass_name}_table[nir_num_search_ops] = {

@@ -1080,7 +1082,8 @@ ${pass_name}_block(nir_builder *build, nir_block *block,
switch (states[alu->dest.dest.ssa.index]) {
  % for i in range(len(automaton.state_patterns)):
case ${i}:
- for (unsigned i = 0; i < ARRAY_SIZE(${pass_name}_state${i}_xforms); 
i++) {
+ % if len(automaton.state_patterns[i]) > 0:
+ for (unsigned i = 0; i < ${len(automaton.state_patterns[i])}; i++) {
  const struct transform *xform = &${pass_name}_state${i}_xforms[i];
  if (condition_flags[xform->condition_offset] &&
  nir_replace_instr(build, alu, xform->search, xform->replace)) 
{
@@ -1088,6 +1091,7 @@ ${pass_name}_block(nir_builder *build, nir_block *block,
 break;
  }
   }
+ % endif
   break;
  % endfor
default: assert(0);



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

Re: [Mesa-dev] [PATCH] nir: don't emit empty initializers for MSVC

[Brian Paul]

On 05/02/2019 01:35 PM, Connor Abbott wrote:
This will crash at runtime, since it'll construct a "struct transform" 
with all NULL pointers, and then the loop below in ${pass_name}_block() 
will see that there's one transform in the array since it uses 
ARRAY_SIZE and then crash trying to access it.


Hmm, any ideas for how to fix this?

-Brian


Running piglit with i965, or radeonsi will reproduce the crash.

On Thu, May 2, 2019 at 7:52 PM Brian Paul <mailto:bri...@vmware.com>> wrote:


This fixes a build failure with MSVC.

---

I've compiled tested this, but not sure how to runtime test it.
---
  src/compiler/nir/nir_algebraic.py | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/compiler/nir/nir_algebraic.py
b/src/compiler/nir/nir_algebraic.py
index 6db749e..dc25421 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -997,6 +997,9 @@ static const struct transform
${pass_name}_state${state_id}_xforms[] = {
  % for i in state_xforms:
    { ${xforms[i].search.c_ptr(cache)},
${xforms[i].replace.c_value_ptr(cache)}, ${xforms[i].condition_index} },
  % endfor
+% if state_xforms == []: # avoid empty initializers for MSVC
+  0
+% endif
  };
  % endfor

-- 
2.7.4


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

<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev=02%7C01%7Cbrianp%40vmware.com%7C9a2c222ad2db4e0696b908d6cf355dca%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636924225451191768=TIk8%2BIqORtPKOvvdeJM2Yo4sMgpk3m97T2WZIKLYdxg%3D=0>



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

[Mesa-dev] [PATCH] nir: don't emit empty initializers for MSVC

[Brian Paul]

This fixes a build failure with MSVC.

---

I've compiled tested this, but not sure how to runtime test it.
---
 src/compiler/nir/nir_algebraic.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/compiler/nir/nir_algebraic.py 
b/src/compiler/nir/nir_algebraic.py
index 6db749e..dc25421 100644
--- a/src/compiler/nir/nir_algebraic.py
+++ b/src/compiler/nir/nir_algebraic.py
@@ -997,6 +997,9 @@ static const struct transform 
${pass_name}_state${state_id}_xforms[] = {
 % for i in state_xforms:
   { ${xforms[i].search.c_ptr(cache)}, ${xforms[i].replace.c_value_ptr(cache)}, 
${xforms[i].condition_index} },
 % endfor
+% if state_xforms == []: # avoid empty initializers for MSVC
+  0
+% endif
 };
 % endfor
 
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] svga: move host logging to winsys

[Brian Paul]

From: Charmaine Lee 

This patch adds a host_log interface to svga_winsys and
moves the host logging code to the winsys layer.

Reviewed-by: Brian Paul 
---
 src/gallium/drivers/svga/Makefile.sources |   2 -
 src/gallium/drivers/svga/meson.build  |   1 -
 src/gallium/drivers/svga/svga_msg.c   | 451 --
 src/gallium/drivers/svga/svga_msg.h   |  42 ---
 src/gallium/drivers/svga/svga_screen.c|  12 +-
 src/gallium/drivers/svga/svga_winsys.h|   5 +
 src/gallium/winsys/svga/drm/Makefile.sources  |   2 +
 src/gallium/winsys/svga/drm/meson.build   |   1 +
 src/gallium/winsys/svga/drm/vmw_msg.c | 448 +
 src/gallium/winsys/svga/drm/vmw_msg.h |  41 +++
 src/gallium/winsys/svga/drm/vmw_screen_svga.c |   3 +
 11 files changed, 506 insertions(+), 502 deletions(-)
 delete mode 100755 src/gallium/drivers/svga/svga_msg.c
 delete mode 100644 src/gallium/drivers/svga/svga_msg.h
 create mode 100644 src/gallium/winsys/svga/drm/vmw_msg.c
 create mode 100644 src/gallium/winsys/svga/drm/vmw_msg.h

diff --git a/src/gallium/drivers/svga/Makefile.sources 
b/src/gallium/drivers/svga/Makefile.sources
index 72024cf..229d286 100644
--- a/src/gallium/drivers/svga/Makefile.sources
+++ b/src/gallium/drivers/svga/Makefile.sources
@@ -15,8 +15,6 @@ C_SOURCES := \
svga_hw_reg.h \
svga_link.c \
svga_link.h \
-   svga_msg.c \
-   svga_msg.h \
svga_mksstats.h \
svga_pipe_blend.c \
svga_pipe_blit.c \
diff --git a/src/gallium/drivers/svga/meson.build 
b/src/gallium/drivers/svga/meson.build
index 7981e29..4d3207a 100644
--- a/src/gallium/drivers/svga/meson.build
+++ b/src/gallium/drivers/svga/meson.build
@@ -27,7 +27,6 @@ files_svga = files(
   'svga_draw_elements.c',
   'svga_format.c',
   'svga_link.c',
-  'svga_msg.c',
   'svga_pipe_blend.c',
   'svga_pipe_blit.c',
   'svga_pipe_clear.c',
diff --git a/src/gallium/drivers/svga/svga_msg.c 
b/src/gallium/drivers/svga/svga_msg.c
deleted file mode 100755
index 8b63132..000
--- a/src/gallium/drivers/svga/svga_msg.c
+++ /dev/null
@@ -1,451 +0,0 @@
-/*
- * Copyright © 2016 VMware, Inc., Palo Alto, CA., USA
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- */
-
-#include "util/u_math.h" /* for MAX2/MIN2 */
-#include "util/u_debug.h"
-#include "util/u_memory.h"
-#include "util/u_string.h"
-#include "pipe/p_defines.h"
-#include "svga_msg.h"
-
-
-#define MESSAGE_STATUS_SUCCESS  0x0001
-#define MESSAGE_STATUS_DORECV   0x0002
-#define MESSAGE_STATUS_CPT  0x0010
-#define MESSAGE_STATUS_HB   0x0080
-
-#define RPCI_PROTOCOL_NUM   0x49435052
-#define GUESTMSG_FLAG_COOKIE0x8000
-
-#define RETRIES 3
-
-#define VMW_HYPERVISOR_MAGIC0x564D5868
-#define VMW_HYPERVISOR_PORT 0x5658
-#define VMW_HYPERVISOR_HB_PORT  0x5659
-
-#define VMW_PORT_CMD_MSG30
-#define VMW_PORT_CMD_HB_MSG 0
-#define VMW_PORT_CMD_OPEN_CHANNEL  (MSG_TYPE_OPEN << 16 | VMW_PORT_CMD_MSG)
-#define VMW_PORT_CMD_CLOSE_CHANNEL (MSG_TYPE_CLOSE << 16 | VMW_PORT_CMD_MSG)
-#define VMW_PORT_CMD_SENDSIZE   (MSG_TYPE_SENDSIZE << 16 | VMW_PORT_CMD_MSG)
-#define VMW_PORT_CMD_RECVSIZE   (MSG_TYPE_RECVSIZE << 16 | VMW_PORT_CMD_MSG)
-#define VMW_PORT_CMD_RECVSTATUS (MSG_TYPE_RECVSTATUS << 16 | VMW_PORT_CMD_MSG)
-
-#define HIGH_WORD(X) ((X & 0x) >> 16)
-
-
-#if defined(PIPE_CC_GCC) && (PIPE_CC_GCC_VERSION > 502)
-
-/**
- * Hypervisor-specific bi-directional communication channel.  Should never
- * execute on bare metal hardware.  The caller must make sure to check for
- * supported hypervisor before using these macros.
- *
- * The last two parameters are both input and output and must be initia

[Mesa-dev] [PATCH 2/2] svga: add SVGA_NO_LOGGING env var (v2)

[Brian Paul]

valgrind crashes when we try to initialize host logging.  This
env var can be used to disable logging.

v2: rebase onto "svga: move host logging to winsys".

Cc: mesa-sta...@lists.freedesktop.org
---
 docs/envvars.html  |  3 +++
 src/gallium/drivers/svga/svga_screen.c | 16 +++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/docs/envvars.html b/docs/envvars.html
index c9733e6..43d3a6c 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -338,6 +338,9 @@ See src/mesa/state_tracker/st_debug.c for other options.
 for details.
 SVGA_EXTRA_LOGGING - if set, enables extra logging to the vmware.log file,
 such as the OpenGL program's name and command line arguments.
+SVGA_NO_LOGGING - if set, disables logging to the vmware.log file.
+This is useful when using Valgrind because it otherwise crashes when
+initializing the host log feature.
 See the driver code for other, lesser-used variables.
 
 
diff --git a/src/gallium/drivers/svga/svga_screen.c 
b/src/gallium/drivers/svga/svga_screen.c
index 664b9bf..f747ff7 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -917,6 +917,16 @@ init_logging(struct pipe_screen *screen)
 }
 
 
+/**
+ * no-op logging function to use when SVGA_NO_LOGGING is set.
+ */
+static void
+nop_host_log(struct svga_winsys_screen *sws, const char *message)
+{
+   /* nothing */
+}
+
+
 static void
 svga_destroy_screen( struct pipe_screen *screen )
 {
@@ -1134,7 +1144,11 @@ svga_screen_create(struct svga_winsys_screen *sws)
 
svga_screen_cache_init(svgascreen);
 
-   init_logging(screen);
+   if (debug_get_bool_option("SVGA_NO_LOGGING", FALSE) == TRUE) {
+  svgascreen->sws->host_log = nop_host_log;
+   } else {
+  init_logging(screen);
+   }
 
return screen;
 error2:
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] glsl: work around MinGW 7.x compiler bug

[Brian Paul]

I'm not sure what triggered this, but building with
scons platform=windows toolchain=crossmingw machine=x86 build=profile
with MinGW g++ 7.3 or 7.4 causes an internal compiler error.

We can work around it by forcing -O1 optimization.
---
 src/compiler/glsl/builtin_variables.cpp | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/compiler/glsl/builtin_variables.cpp 
b/src/compiler/glsl/builtin_variables.cpp
index 17ee80c..1b9963a 100644
--- a/src/compiler/glsl/builtin_variables.cpp
+++ b/src/compiler/glsl/builtin_variables.cpp
@@ -21,6 +21,21 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+
+/**
+ * Building this file with MinGW g++ 7.3 or 7.4 with:
+ *   scons platform=windows toolchain=crossmingw machine=x86 build=profile
+ * triggers an internal compiler error.
+ * Overriding the optimization level to -O1 works around the issue.
+ * MinGW 5.3.1 does not seem to have the bug, neither does 8.3.  So for now
+ * we're simply testing for version 7.x here.
+ */
+#if defined(__MINGW32__) && __GNUC__ == 7
+#warning "disabling optimizations for this file to work around compiler bug in 
MiGW gcc 7.x"
+#pragma GCC optimize("O1")
+#endif
+
+
 #include "ir.h"
 #include "ir_builder.h"
 #include "linker.h"
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] llvmpipe: init some vars to NULL to silence MinGW compiler warnings

[Brian Paul]

---
 src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c 
b/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
index 9561c34..90b2be9 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_format_s3tc.c
@@ -2191,7 +2191,7 @@ lp_build_fetch_s3tc_rgba_aos(struct gallivm_state 
*gallivm,
   rgba = LLVMGetUndef(i128_vectype);
 
   for (count = 0; count < n / 4; count++) {
- LLVMValueRef colors, codewords, alpha_lo, alpha_hi;
+ LLVMValueRef colors, codewords, alpha_lo = NULL, alpha_hi = NULL;
 
  i4 = lp_build_extract_range(gallivm, i, count * 4, 4);
  j4 = lp_build_extract_range(gallivm, j, count * 4, 4);
@@ -2230,7 +2230,7 @@ lp_build_fetch_s3tc_rgba_aos(struct gallivm_state 
*gallivm,
   rgba = LLVMBuildBitCast(builder, rgba, i8_vectype, "");
}
else {
-  LLVMValueRef colors, codewords, alpha_lo, alpha_hi;
+  LLVMValueRef colors, codewords, alpha_lo = NULL, alpha_hi = NULL;
 
   lp_build_gather_s3tc(gallivm, n, format_desc, , ,
_lo, _hi, base_ptr, offset);
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] svga: add SVGA_NO_LOGGING env var

[Brian Paul]

valgrind crashes when we try to initialize host logging.  This
env var can be used to disable logging.

Cc: mesa-sta...@lists.freedesktop.org
---
 docs/envvars.html  | 3 +++
 src/gallium/drivers/svga/svga_screen.c | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/docs/envvars.html b/docs/envvars.html
index c9733e6..43d3a6c 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -338,6 +338,9 @@ See src/mesa/state_tracker/st_debug.c for other options.
 for details.
 SVGA_EXTRA_LOGGING - if set, enables extra logging to the vmware.log file,
 such as the OpenGL program's name and command line arguments.
+SVGA_NO_LOGGING - if set, disables logging to the vmware.log file.
+This is useful when using Valgrind because it otherwise crashes when
+initializing the host log feature.
 See the driver code for other, lesser-used variables.
 
 
diff --git a/src/gallium/drivers/svga/svga_screen.c 
b/src/gallium/drivers/svga/svga_screen.c
index 6cb5a14..8158950 100644
--- a/src/gallium/drivers/svga/svga_screen.c
+++ b/src/gallium/drivers/svga/svga_screen.c
@@ -1134,7 +1134,9 @@ svga_screen_create(struct svga_winsys_screen *sws)
 
svga_screen_cache_init(svgascreen);
 
-   init_logging(screen);
+   if (debug_get_bool_option("SVGA_NO_LOGGING", FALSE) == FALSE) {
+  init_logging(screen);
+   }
 
return screen;
 error2:
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH 1/5] winsys/svga: Add an environment variable to force host-backed operation

[Brian Paul]

The series looks good to me.

Reviewed-by: Brian Paul 

On 04/30/2019 05:04 AM, Thomas Hellstrom wrote:

The vmwgfx kernel module has a compatibility mode for user-space that is
not guest-backed resource aware. Add an environment variable to facilitate
testing of this mode on guest-backed aware kernels: if the environment
variable SVGA_FORCE_HOST_BACKED is defined, the driver will use host-backed
operation.

Signed-off-by: Thomas Hellstrom 
Reviewed-by: Deepak Rawat 
---
  src/gallium/winsys/svga/drm/vmw_screen_ioctl.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c 
b/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
index 0ec8c1abe11..cdfe284c4c8 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
@@ -967,6 +967,7 @@ vmw_ioctl_init(struct vmw_winsys_screen *vws)
 drmVersionPtr version;
 boolean drm_gb_capable;
 boolean have_drm_2_5;
+   const char *getenv_val;
  
 VMW_FUNC;
  
@@ -1006,17 +1007,21 @@ vmw_ioctl_init(struct vmw_winsys_screen *vws)

goto out_no_3d;
 }
 vws->ioctl.hwversion = gp_arg.value;
-
-   memset(_arg, 0, sizeof(gp_arg));
-   gp_arg.param = DRM_VMW_PARAM_HW_CAPS;
-   ret = drmCommandWriteRead(vws->ioctl.drm_fd, DRM_VMW_GET_PARAM,
- _arg, sizeof(gp_arg));
+   getenv_val = getenv("SVGA_FORCE_HOST_BACKED");
+   if (!getenv_val || strcmp(getenv_val, "0") == 0) {
+  memset(_arg, 0, sizeof(gp_arg));
+  gp_arg.param = DRM_VMW_PARAM_HW_CAPS;
+  ret = drmCommandWriteRead(vws->ioctl.drm_fd, DRM_VMW_GET_PARAM,
+_arg, sizeof(gp_arg));
+   } else {
+  ret = -EINVAL;
+   }
 if (ret)
vws->base.have_gb_objects = FALSE;
 else
vws->base.have_gb_objects =
   !!(gp_arg.value & (uint64_t) SVGA_CAP_GBOBJECTS);
-
+
 if (vws->base.have_gb_objects && !drm_gb_capable)
goto out_no_3d;
  



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

Re: [Mesa-dev] [PATCH 1/3] pipebuffer: use new pb_usage_flags enum type

[Brian Paul]

It was committed in early March: b286e74df66e25cadd1c82d9ddc4d1fc3887b646

-Brian

On 04/23/2019 07:25 AM, Thomas Hellstrom wrote:

Hi, Brian,

Did this series get reviewed? I don't see any replies?

/Thomas


On Tue, 2019-03-05 at 20:48 -0700, Brian Paul wrote:

Use a new enum type instead of 'unsigned' to make things a bit more
understandable.
---
  src/gallium/auxiliary/pipebuffer/pb_buffer.h   | 34
++
  .../auxiliary/pipebuffer/pb_buffer_fenced.c|  6 ++--
  .../auxiliary/pipebuffer/pb_buffer_malloc.c|  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c |  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_debug.c |  6 ++--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_mm.c|  4 +--
  .../auxiliary/pipebuffer/pb_bufmgr_ondemand.c  |  4 +--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_pool.c  |  5 ++--
  src/gallium/auxiliary/pipebuffer/pb_bufmgr_slab.c  |  6 ++--
  src/gallium/auxiliary/pipebuffer/pb_validate.c |  2 +-
  src/gallium/auxiliary/pipebuffer/pb_validate.h |  2 +-
  11 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
index 33c2306..11f70ea 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_buffer.h
+++ b/src/gallium/auxiliary/pipebuffer/pb_buffer.h
@@ -59,13 +59,22 @@ struct pb_vtbl;
  struct pb_validate;
  struct pipe_fence_handle;
  
+enum pb_usage_flags {

+   PB_USAGE_CPU_READ = (1 << 0),
+   PB_USAGE_CPU_WRITE = (1 << 1),
+   PB_USAGE_GPU_READ = (1 << 2),
+   PB_USAGE_GPU_WRITE = (1 << 3),
+   PB_USAGE_DONTBLOCK = (1 << 9),
+   PB_USAGE_UNSYNCHRONIZED = (1 << 10),
+};
  
-#define PB_USAGE_CPU_READ  (1 << 0)

-#define PB_USAGE_CPU_WRITE (1 << 1)
-#define PB_USAGE_GPU_READ  (1 << 2)
-#define PB_USAGE_GPU_WRITE (1 << 3)
-#define PB_USAGE_UNSYNCHRONIZED (1 << 10)
-#define PB_USAGE_DONTBLOCK (1 << 9)
+/* For error checking elsewhere */
+#define PB_USAGE_ALL (PB_USAGE_CPU_READ | \
+  PB_USAGE_CPU_WRITE | \
+  PB_USAGE_GPU_READ | \
+  PB_USAGE_GPU_WRITE | \
+  PB_USAGE_DONTBLOCK | \
+  PB_USAGE_UNSYNCHRONIZED)
  
  #define PB_USAGE_CPU_READ_WRITE \

 ( PB_USAGE_CPU_READ | PB_USAGE_CPU_WRITE )
@@ -82,7 +91,7 @@ struct pipe_fence_handle;
  struct pb_desc
  {
 unsigned alignment;
-   unsigned usage;
+   enum pb_usage_flags usage;
  };
  
  
@@ -100,7 +109,7 @@ struct pb_buffer

 struct pipe_reference  reference;
 unsigned   alignment;
 pb_sizesize;
-   unsigned   usage;
+   enum pb_usage_flagsusage;
  
 /**

  * Pointer to the virtual function table.
@@ -126,13 +135,13 @@ struct pb_vtbl
  * flags is bitmask of PB_USAGE_CPU_READ/WRITE.
  */
 void *(*map)( struct pb_buffer *buf,
- unsigned flags, void *flush_ctx );
+ enum pb_usage_flags flags, void *flush_ctx );
 
 void (*unmap)( struct pb_buffer *buf );
  
 enum pipe_error (*validate)( struct pb_buffer *buf,

  struct pb_validate *vl,
-unsigned flags );
+enum pb_usage_flags flags );
  
 void (*fence)( struct pb_buffer *buf,

struct pipe_fence_handle *fence );
@@ -160,7 +169,7 @@ struct pb_vtbl
   */
  static inline void *
  pb_map(struct pb_buffer *buf,
-   unsigned flags, void *flush_ctx)
+   enum pb_usage_flags flags, void *flush_ctx)
  {
 assert(buf);
 if (!buf)
@@ -201,7 +210,8 @@ pb_get_base_buffer( struct pb_buffer *buf,
  
  
  static inline enum pipe_error

-pb_validate(struct pb_buffer *buf, struct pb_validate *vl, unsigned
flags)
+pb_validate(struct pb_buffer *buf, struct pb_validate *vl,
+enum pb_usage_flags flags)
  {
 assert(buf);
 if (!buf)
diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
index 7421741..53b9ce0 100644
--- a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
+++ b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c
@@ -139,7 +139,7 @@ struct fenced_buffer
  * A bitmask of PB_USAGE_CPU/GPU_READ/WRITE describing the
current
  * buffer usage.
  */
-   unsigned flags;
+   enum pb_usage_flags flags;
  
 unsigned mapcount;
  
@@ -662,7 +662,7 @@ fenced_buffer_destroy(struct pb_buffer *buf)
  
  static void *

  fenced_buffer_map(struct pb_buffer *buf,
-  unsigned flags, void *flush_ctx)
+  enum pb_usage_flags flags, void *flush_ctx)
  {
 struct fenced_buffer *fenced_buf = fenced_buffer(buf);
 struct fenced_manager *fenced_mgr = fenced_buf->mgr;
@@ -739,7 +739,7 @@ fenced_buffer_unmap(struct pb_buffer *buf)
  static enum pipe_error
  fenced_buffer_validate(struct pb_buffer *buf,
  

Re: [Mesa-dev] [PATCH] gallivm: fix saturated signed add / sub with llvm 9

[Brian Paul]

On 04/16/2019 06:35 PM, srol...@vmware.com wrote:

From: Roland Scheidegger 

llvm 8 removed saturated unsigned add / sub x86 sse2 intrinsics, and
now llvm 9 removed the signed versions as well - they were proposed for
removal earlier, but the pattern to recognize those was very complex,
so it wasn't done then. However, instead of these arch-specific
intrinsics, there's now arch-independent intrinsics for saturated
add / sub, both for signed and unsigned, so use these.
They should have only advantages (work with arbitrary vector sizes,
optimal code for all archs), although I don't know how well they work
in practice for other archs (at least for x86 they do the right thing).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110454
---
  src/gallium/auxiliary/gallivm/lp_bld_arit.c | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index 057c50ed278..02fb81afe51 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -555,6 +555,12 @@ lp_build_add(struct lp_build_context *bld,
  return bld->one;
  
if (!type.floating && !type.fixed) {

+ if (HAVE_LLVM >= 0x0900) {
+char intrin[32];
+intrinsic = type.sign ? "llvm.sadd.sat" : "llvm.uadd.sat";
+lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
+return lp_build_intrinsic_binary(builder, intrin, bld->vec_type, 
a, b);
+ }
   if (type.width * type.length == 128) {
  if (util_cpu_caps.has_sse2) {
 if (type.width == 8)
@@ -625,6 +631,7 @@ lp_build_add(struct lp_build_context *bld,
* NOTE: cmp/select does sext/trunc of the mask. Does not seem to
* interfere with llvm's ability to recognize the pattern but seems
* a bit brittle.
+  * NOTE: llvm 9+ always uses (non arch specific) intrinsic.
*/
   LLVMValueRef overflowed = lp_build_cmp(bld, PIPE_FUNC_GREATER, a, 
res);
   res = lp_build_select(bld, overflowed,
@@ -876,6 +883,12 @@ lp_build_sub(struct lp_build_context *bld,
  return bld->zero;
  
if (!type.floating && !type.fixed) {

+ if (HAVE_LLVM >= 0x0900) {
+char intrin[32];
+intrinsic = type.sign ? "llvm.ssub.sat" : "llvm.usub.sat";
+lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
+return lp_build_intrinsic_binary(builder, intrin, bld->vec_type, 
a, b);
+ }
   if (type.width * type.length == 128) {
  if (util_cpu_caps.has_sse2) {
 if (type.width == 8)
@@ -925,6 +938,7 @@ lp_build_sub(struct lp_build_context *bld,
* NOTE: cmp/select does sext/trunc of the mask. Does not seem to
* interfere with llvm's ability to recognize the pattern but seems
* a bit brittle.
+  * NOTE: llvm 9+ always uses (non arch specific) intrinsic.
*/
   LLVMValueRef no_ov = lp_build_cmp(bld, PIPE_FUNC_GREATER, a, b);
   a = lp_build_select(bld, no_ov, a, b);



LGTM.

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

Re: [Mesa-dev] [PATCH] gallivm: fix bogus assert in get_indirect_index

[Brian Paul]

On 04/15/2019 01:39 PM, srol...@vmware.com wrote:

From: Roland Scheidegger 

0 is a valid value as max index, and the code handles it fine. This isn't
commonly seen, as it will only happen with array declarations of size 1.
The assert was introduced with a3c898dc97ec5f0e0b93b2ee180bdf8ca3bab14c.

Fixes piglit tests/shaders/complex-loop-analysis-bug.shader_test
Bugzilla: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D110441data=02%7C01%7Cbrianp%40vmware.com%7Cc4b58aeaf4c245f7794e08d6c1da2226%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636909539954259111sdata=l%2BScozK4SM92MrI4bIo7CBa1HujD0OFeKDz9ixvUdI8%3Dreserved=0
---
  src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
index 0f5b3d9acb7..d6af1d84471 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
@@ -1108,7 +1108,7 @@ get_indirect_index(struct lp_build_tgsi_soa_context *bld,
  * larger than the declared size but smaller than the buffer size.
  */
 if (reg_file != TGSI_FILE_CONSTANT) {
-  assert(index_limit > 0);
+  assert(index_limit >= 0);
max_index = lp_build_const_int_vec(bld->bld_base.base.gallivm,
   uint_bld->type, index_limit);
  



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

Re: [Mesa-dev] [PATCH] [rfc] st/mesa: don't update fb state is raster discard is set.

[Brian Paul]

On 04/10/2019 12:10 AM, Dave Airlie wrote:

From: Dave Airlie 

This avoid softpipe trying to get image when no window has ever
been exposed, and no image will be exposed.

I'm not entirely sure this is correct or useful, but it definitely
helps with some softpipe get images when we haven't got a window.
---
  src/mesa/state_tracker/st_atom.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/mesa/state_tracker/st_atom.c b/src/mesa/state_tracker/st_atom.c
index 49f79ad9d49..3aeb526e56b 100644
--- a/src/mesa/state_tracker/st_atom.c
+++ b/src/mesa/state_tracker/st_atom.c
@@ -246,6 +246,9 @@ void st_validate_state( struct st_context *st, enum 
st_pipeline pipeline )
unreachable("Invalid pipeline specified");
 }
  
+   if (ctx->RasterDiscard)

+  st->dirty &= ~ST_NEW_FB_STATE;
+
 dirty = st->dirty & pipeline_mask;
 if (!dirty)
return;



Yeah, this looks a little questionable.

Couldn't you add a check for ctx->RasterDiscard in 
st_update_framebuffer_state() and no-op most of that function there?


I think you'd at least want to do this after flushing the bitmap and 
readpix cache.


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

Re: [Mesa-dev] [PATCH] haiku: Fix hgl dispatch build. Tested under meson/scons.

[Brian Paul]

On Sat, Mar 30, 2019 at 6:30 PM Alexander von Gluck IV <
kallis...@unixzen.com> wrote:

> ---
>  src/hgl/GLDispatcher.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/hgl/GLDispatcher.h b/src/hgl/GLDispatcher.h
> index 8aaf58a6237..7a4bcd33299 100644
> --- a/src/hgl/GLDispatcher.h
> +++ b/src/hgl/GLDispatcher.h
> @@ -15,7 +15,7 @@
>  #include 
>  #include 
>
> -#include "glheader.h"
> +#include "main/glheader.h"
>
>  #include "glapi/glapi.h"
>
> --


Looks OK to me.   Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] util: no-op __builtin_types_compatible_p() for non-GCC compilers

[Brian Paul]

On 03/29/2019 12:58 PM, Ian Romanick wrote:

On 3/29/19 9:57 AM, Brian Paul wrote:

__builtin_types_compatible_p() is GCC-specific and breaks the
MSVC build.

This intrinsic has been in u_vector_foreach() for a long time, but
that macro has only recently been used in code
(nir/nir_opt_comparison_pre.c) that's built with MSVC.

Fixes: 2cf59861a ("nir: Add partial redundancy elimination for compares")
---
  src/util/u_vector.h | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/util/u_vector.h b/src/util/u_vector.h
index cd8a95d..6807748 100644
--- a/src/util/u_vector.h
+++ b/src/util/u_vector.h
@@ -80,6 +80,10 @@ u_vector_finish(struct u_vector *queue)
 free(queue->data);
  }
  
+#ifndef __GNUC__

+#define __builtin_types_compatible_p(x) 1
+#endif
+
  #define u_vector_foreach(elem, queue)  \
 STATIC_ASSERT(__builtin_types_compatible_p(__typeof__(queue), struct 
u_vector *)); \


The way this is GCC builtin is used here, this should be fine.  However,
in case it's begin used elsewhere, we should #undef it afterwards.



That doesn't seem to work.  When u_vector_foreach() is instantiated 
later, __builtin_types_compatible_p is undefined and we error out.


-Brian


  I'd

hate to mask some other kind of bug that may be introduced later.


 for (uint32_t __u_vector_offset = (queue)->tail;   
 \





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

[Mesa-dev] [PATCH] util: no-op __builtin_types_compatible_p() for non-GCC compilers

[Brian Paul]

__builtin_types_compatible_p() is GCC-specific and breaks the
MSVC build.

This intrinsic has been in u_vector_foreach() for a long time, but
that macro has only recently been used in code
(nir/nir_opt_comparison_pre.c) that's built with MSVC.

Fixes: 2cf59861a ("nir: Add partial redundancy elimination for compares")
---
 src/util/u_vector.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/util/u_vector.h b/src/util/u_vector.h
index cd8a95d..6807748 100644
--- a/src/util/u_vector.h
+++ b/src/util/u_vector.h
@@ -80,6 +80,10 @@ u_vector_finish(struct u_vector *queue)
free(queue->data);
 }
 
+#ifndef __GNUC__
+#define __builtin_types_compatible_p(x) 1
+#endif
+
 #define u_vector_foreach(elem, queue)  \
STATIC_ASSERT(__builtin_types_compatible_p(__typeof__(queue), struct 
u_vector *)); \
for (uint32_t __u_vector_offset = (queue)->tail;
\
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH] softpipe: fix clears to only clear specified color buffers.

[Brian Paul]

On 03/26/2019 12:44 AM, Dave Airlie wrote:

From: Dave Airlie 

This fixes piglit clearbuffer-mixed-format
---
  src/gallium/drivers/softpipe/sp_clear.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/softpipe/sp_clear.c 
b/src/gallium/drivers/softpipe/sp_clear.c
index ba04fd0aa87..d2626a24333 100644
--- a/src/gallium/drivers/softpipe/sp_clear.c
+++ b/src/gallium/drivers/softpipe/sp_clear.c
@@ -68,7 +68,8 @@ softpipe_clear(struct pipe_context *pipe, unsigned buffers,
  
 if (buffers & PIPE_CLEAR_COLOR) {

for (i = 0; i < softpipe->framebuffer.nr_cbufs; i++) {
- sp_tile_cache_clear(softpipe->cbuf_cache[i], color, 0);
+ if (buffers & (PIPE_CLEAR_COLOR0 << i))
+sp_tile_cache_clear(softpipe->cbuf_cache[i], color, 0);
}
 }
  



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

Re: [Mesa-dev] [PATCH] [rfc] draw/vs: partly fix basevertex/vertex id

[Brian Paul]

On 03/25/2019 06:47 PM, Dave Airlie wrote:

From: Dave Airlie 

This gets the basevertex from the draw depending on whether
it's an indexed or non-indexed draw.

We still fail a transform feedback test for vertex id, as
the vertex id actually an index id, and isn't getting translated
properly to a vertex id, suggestions on how/where to fix that welcome.
---
  src/gallium/auxiliary/draw/draw_vs_exec.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_vs_exec.c 
b/src/gallium/auxiliary/draw/draw_vs_exec.c
index 4f11ac7506c..dbd7aa551eb 100644
--- a/src/gallium/auxiliary/draw/draw_vs_exec.c
+++ b/src/gallium/auxiliary/draw/draw_vs_exec.c
@@ -128,18 +128,17 @@ vs_exec_run_linear(struct draw_vertex_shader *shader,
   input[slot][3]);
   }
  #endif
+int basevertex = shader->draw->pt.user.eltSize ? 
shader->draw->pt.user.eltBias : shader->draw->start_index;
  
   if (shader->info.uses_vertexid) {

  unsigned vid = 
machine->SysSemanticToIndex[TGSI_SEMANTIC_VERTEXID];
  assert(vid < ARRAY_SIZE(machine->SystemValue));
-machine->SystemValue[vid].xyzw[0].i[j] = i + j;
-/* XXX this should include base vertex. Where to get it??? */
+machine->SystemValue[vid].xyzw[0].i[j] = i + j + basevertex;
   }
   if (shader->info.uses_basevertex) {
  unsigned vid = 
machine->SysSemanticToIndex[TGSI_SEMANTIC_BASEVERTEX];
  assert(vid < ARRAY_SIZE(machine->SystemValue));
-machine->SystemValue[vid].xyzw[0].i[j] = 0;
-/* XXX Where to get it??? */
+machine->SystemValue[vid].xyzw[0].i[j] = basevertex;
   }
   if (shader->info.uses_vertexid_nobase) {
  unsigned vid = 
machine->SysSemanticToIndex[TGSI_SEMANTIC_VERTEXID_NOBASE];



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

Re: [Mesa-dev] [PATCH] st/mesa: fix texture deletion context mix-up issues (v2)

[Brian Paul]

On 03/23/2019 10:49 AM, Jose Fonseca wrote:

Looks good to me.

Reviewed-by: Jose Fonseca 

Though I wonder if this could happen also when not destroying the 
current context. (Ie, if we need zoombie textures too?)


If we're not destroying the thread's current context, this patch 
temporarily binds the context as the current one.  If the contexts 
textures are not shared, they'll be deleted.  If they are shared, they 
won't be deleted.  I think that part is fairly straight-forward.


-Brian




Jose



*From:* Brian Paul 
*Sent:* Friday, March 22, 2019 19:51
*To:* mesa-dev@lists.freedesktop.org
*Cc:* Jose Fonseca; Neha Bhende
*Subject:* [PATCH] st/mesa: fix texture deletion context mix-up issues (v2)
When we destroy a context, we need to temporarily make that context
the current one for the thread.

That's because during context tear-down we make many calls to
_mesa_reference_texobj(, NULL).  Note there's no context
parameter.  If the texture's refcount goes to zero and we need to
delete it, we use the thread's current context.  But if that context
isn't the context we're tearing down, we get into trouble when
deallocating sampler views.  See patch 593e36f956 ("st/mesa:
implement "zombie" sampler views (v2)") for background information.

Also, we need to release any sampler views attached to the fallback
textures.

Fixes a crash on exit with a glretrace of the Nobel Clinician
application.

v2: at end of st_destroy_context(), check if save_ctx == ctx and
unbind the context if so.
---
  src/mesa/state_tracker/st_context.c | 51 
-

  1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c

index f037384..09d467a 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -917,15 +917,39 @@ st_destroy_context(struct st_context *st)
  {
     struct gl_context *ctx = st->ctx;
     struct st_framebuffer *stfb, *next;
+   struct gl_framebuffer *save_drawbuffer;
+   struct gl_framebuffer *save_readbuffer;
+
+   /* Save the current context and draw/read buffers*/
+   GET_CURRENT_CONTEXT(save_ctx);
+   if (save_ctx) {
+  save_drawbuffer = save_ctx->WinSysDrawBuffer;
+  save_readbuffer = save_ctx->WinSysReadBuffer;
+   } else {
+  save_drawbuffer = save_readbuffer = NULL;
+   }

-   GET_CURRENT_CONTEXT(curctx);
+   /*
+    * We need to bind the context we're deleting so that
+    * _mesa_reference_texobj_() uses this context when deleting textures.
+    * Similarly for framebuffer objects, etc.
+    */
+   _mesa_make_current(ctx, NULL, NULL);

-   if (curctx == NULL) {
-  /* No current context, but we need one to release
-   * renderbuffer surface when we release framebuffer.
-   * So temporarily bind the context.
-   */
-  _mesa_make_current(ctx, NULL, NULL);
+   /* This must be called first so that glthread has a chance to finish */
+   _mesa_glthread_destroy(ctx);
+
+   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
+
+   /* For the fallback textures, free any sampler views belonging to this
+    * context.
+    */
+   for (unsigned i = 0; i < NUM_TEXTURE_TARGETS; i++) {
+  struct st_texture_object *stObj =
+ st_texture_object(ctx->Shared->FallbackTex[i]);
+  if (stObj) {
+ st_texture_release_context_sampler_view(st, stObj);
+  }
     }

     st_context_free_zombie_objects(st);
@@ -933,11 +957,6 @@ st_destroy_context(struct st_context *st)
     mtx_destroy(>zombie_sampler_views.mutex);
     mtx_destroy(>zombie_shaders.mutex);

-   /* This must be called first so that glthread has a chance to finish */
-   _mesa_glthread_destroy(ctx);
-
-   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
-
     st_reference_fragprog(st, >fp, NULL);
     st_reference_prog(st, >gp, NULL);
     st_reference_vertprog(st, >vp, NULL);
@@ -965,4 +984,12 @@ st_destroy_context(struct st_context *st)
     st = NULL;

     free(ctx);
+
+   if (save_ctx == ctx) {
+  /* unbind the context we just deleted */
+  _mesa_make_current(NULL, NULL, NULL);
+   } else {
+  /* Restore the current context and draw/read buffers (may be NULL) */
+  _mesa_make_current(save_ctx, save_drawbuffer, save_readbuffer);
+   }
  }
--
1.8.5.6



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

Re: [Mesa-dev] [PATCH] draw/gs: fix point size outputs from geometry shader.

[Brian Paul]

On 03/24/2019 11:14 PM, Dave Airlie wrote:

From: Dave Airlie 

If the geom shader emits a point size we failed to find it here,
use the correct API to look it up.

Fixes:
tests/spec/glsl-1.50/execution/geometry/point-size-out.shader_test
---
  src/gallium/auxiliary/draw/draw_pipe_wide_point.c | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pipe_wide_point.c 
b/src/gallium/auxiliary/draw/draw_pipe_wide_point.c
index 1329ab4e7c0..e9bbb67a958 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_wide_point.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_wide_point.c
@@ -266,14 +266,7 @@ widepoint_first_point(struct draw_stage *stage,
 wide->psize_slot = -1;
 if (rast->point_size_per_vertex) {
/* find PSIZ vertex output */
-  const struct draw_vertex_shader *vs = draw->vs.vertex_shader;
-  uint i;
-  for (i = 0; i < vs->info.num_outputs; i++) {
- if (vs->info.output_semantic_name[i] == TGSI_SEMANTIC_PSIZE) {
-wide->psize_slot = i;
-break;
- }
-  }
+  wide->psize_slot = draw_find_shader_output(draw, TGSI_SEMANTIC_PSIZE, 0);
 }
 
 stage->point( stage, header );




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

Re: [Mesa-dev] [PATCH] draw: bail instead of assert on instance count (v2)

[Brian Paul]

On 03/24/2019 09:41 PM, Dave Airlie wrote:

From: Dave Airlie 

With indirect rendering it's fine to set the instance count
parameter to 0, and expect the rendering to be ignored.

Fixes assert in KHR-GLES31.core.compute_shader.pipeline-gen-draw-commands
on softpipe

v2: return earlier before changing fpstate
---
  src/gallium/auxiliary/draw/draw_pt.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/draw/draw_pt.c 
b/src/gallium/auxiliary/draw/draw_pt.c
index be76a30f97c..50286149cd4 100644
--- a/src/gallium/auxiliary/draw/draw_pt.c
+++ b/src/gallium/auxiliary/draw/draw_pt.c
@@ -464,6 +464,9 @@ draw_vbo(struct draw_context *draw,
 unsigned fpstate = util_fpstate_get();
 struct pipe_draw_info resolved_info;
  
+   if (info->instance_count == 0)

+  return;
+
 /* Make sure that denorms are treated like zeros. This is
  * the behavior required by D3D10. OpenGL doesn't care.
  */
@@ -472,7 +475,6 @@ draw_vbo(struct draw_context *draw,
 resolve_draw_info(info, _info, &(draw->pt.vertex_buffer[0]));
 info = _info;
  
-   assert(info->instance_count > 0);

 if (info->index_size)
assert(draw->pt.user.elts);
  



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

[Mesa-dev] [PATCH] st/mesa: fix texture deletion context mix-up issues (v2)

[Brian Paul]

When we destroy a context, we need to temporarily make that context
the current one for the thread.

That's because during context tear-down we make many calls to
_mesa_reference_texobj(, NULL).  Note there's no context
parameter.  If the texture's refcount goes to zero and we need to
delete it, we use the thread's current context.  But if that context
isn't the context we're tearing down, we get into trouble when
deallocating sampler views.  See patch 593e36f956 ("st/mesa:
implement "zombie" sampler views (v2)") for background information.

Also, we need to release any sampler views attached to the fallback
textures.

Fixes a crash on exit with a glretrace of the Nobel Clinician
application.

v2: at end of st_destroy_context(), check if save_ctx == ctx and
unbind the context if so.
---
 src/mesa/state_tracker/st_context.c | 51 -
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index f037384..09d467a 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -917,15 +917,39 @@ st_destroy_context(struct st_context *st)
 {
struct gl_context *ctx = st->ctx;
struct st_framebuffer *stfb, *next;
+   struct gl_framebuffer *save_drawbuffer;
+   struct gl_framebuffer *save_readbuffer;
+
+   /* Save the current context and draw/read buffers*/
+   GET_CURRENT_CONTEXT(save_ctx);
+   if (save_ctx) {
+  save_drawbuffer = save_ctx->WinSysDrawBuffer;
+  save_readbuffer = save_ctx->WinSysReadBuffer;
+   } else {
+  save_drawbuffer = save_readbuffer = NULL;
+   }
 
-   GET_CURRENT_CONTEXT(curctx);
+   /*
+* We need to bind the context we're deleting so that
+* _mesa_reference_texobj_() uses this context when deleting textures.
+* Similarly for framebuffer objects, etc.
+*/
+   _mesa_make_current(ctx, NULL, NULL);
 
-   if (curctx == NULL) {
-  /* No current context, but we need one to release
-   * renderbuffer surface when we release framebuffer.
-   * So temporarily bind the context.
-   */
-  _mesa_make_current(ctx, NULL, NULL);
+   /* This must be called first so that glthread has a chance to finish */
+   _mesa_glthread_destroy(ctx);
+
+   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
+
+   /* For the fallback textures, free any sampler views belonging to this
+* context.
+*/
+   for (unsigned i = 0; i < NUM_TEXTURE_TARGETS; i++) {
+  struct st_texture_object *stObj =
+ st_texture_object(ctx->Shared->FallbackTex[i]);
+  if (stObj) {
+ st_texture_release_context_sampler_view(st, stObj);
+  }
}
 
st_context_free_zombie_objects(st);
@@ -933,11 +957,6 @@ st_destroy_context(struct st_context *st)
mtx_destroy(>zombie_sampler_views.mutex);
mtx_destroy(>zombie_shaders.mutex);
 
-   /* This must be called first so that glthread has a chance to finish */
-   _mesa_glthread_destroy(ctx);
-
-   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
-
st_reference_fragprog(st, >fp, NULL);
st_reference_prog(st, >gp, NULL);
st_reference_vertprog(st, >vp, NULL);
@@ -965,4 +984,12 @@ st_destroy_context(struct st_context *st)
st = NULL;
 
free(ctx);
+
+   if (save_ctx == ctx) {
+  /* unbind the context we just deleted */
+  _mesa_make_current(NULL, NULL, NULL);
+   } else {
+  /* Restore the current context and draw/read buffers (may be NULL) */
+  _mesa_make_current(save_ctx, save_drawbuffer, save_readbuffer);
+   }
 }
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] nir: fix a few signed/unsigned comparison warnings

[Brian Paul]

---
 src/compiler/nir_types.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir_types.cpp b/src/compiler/nir_types.cpp
index aff575c..3bf93c5 100644
--- a/src/compiler/nir_types.cpp
+++ b/src/compiler/nir_types.cpp
@@ -690,7 +690,7 @@ glsl_type_get_sampler_count(const struct glsl_type *type)
 
if (glsl_type_is_struct_or_ifc(type)) {
   unsigned count = 0;
-  for (int i = 0; i < glsl_get_length(type); i++)
+  for (unsigned i = 0; i < glsl_get_length(type); i++)
  count += glsl_type_get_sampler_count(glsl_get_struct_field(type, i));
   return count;
}
@@ -711,7 +711,7 @@ glsl_type_get_image_count(const struct glsl_type *type)
 
if (glsl_type_is_struct_or_ifc(type)) {
   unsigned count = 0;
-  for (int i = 0; i < glsl_get_length(type); i++)
+  for (unsigned i = 0; i < glsl_get_length(type); i++)
  count += glsl_type_get_image_count(glsl_get_struct_field(type, i));
   return count;
}
-- 
1.8.5.6

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

[Mesa-dev] [PATCH] st/mesa: fix texture deletion context mix-up issues

[Brian Paul]

When we destroy a context, we need to temporarily make that context
the current one for the thread.

That's because during context tear-down we make many calls to
_mesa_reference_texobj(, NULL).  Note there's no context
parameter.  If the texture's refcount goes to zero and we need to
delete it, we use the thread's current context.  But if that context
isn't the context we're tearing down, we get into trouble when
deallocating sampler views.  See patch 593e36f956 ("st/mesa:
implement "zombie" sampler views (v2)") for background information.

Also, we need to release any sampler views attached to the fallback
textures.

Fixes a crash on exit with a glretrace of the Nobel Clinician
application.
---
 src/mesa/state_tracker/st_context.c | 46 +++--
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index f037384..9b23d75 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -917,15 +917,39 @@ st_destroy_context(struct st_context *st)
 {
struct gl_context *ctx = st->ctx;
struct st_framebuffer *stfb, *next;
+   struct gl_framebuffer *save_drawbuffer;
+   struct gl_framebuffer *save_readbuffer;
+
+   /* Save the current context and draw/read buffers*/
+   GET_CURRENT_CONTEXT(save_ctx);
+   if (save_ctx) {
+  save_drawbuffer = save_ctx->WinSysDrawBuffer;
+  save_readbuffer = save_ctx->WinSysReadBuffer;
+   } else {
+  save_drawbuffer = save_readbuffer = NULL;
+   }
 
-   GET_CURRENT_CONTEXT(curctx);
+   /*
+* We need to bind the context we're deleting so that
+* _mesa_reference_texobj_() uses this context when deleting textures.
+* Similarly for framebuffer objects, etc.
+*/
+   _mesa_make_current(ctx, NULL, NULL);
 
-   if (curctx == NULL) {
-  /* No current context, but we need one to release
-   * renderbuffer surface when we release framebuffer.
-   * So temporarily bind the context.
-   */
-  _mesa_make_current(ctx, NULL, NULL);
+   /* This must be called first so that glthread has a chance to finish */
+   _mesa_glthread_destroy(ctx);
+
+   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
+
+   /* For the fallback textures, free any sampler views belonging to this
+* context.
+*/
+   for (unsigned i = 0; i < NUM_TEXTURE_TARGETS; i++) {
+  struct st_texture_object *stObj =
+ st_texture_object(ctx->Shared->FallbackTex[i]);
+  if (stObj) {
+ st_texture_release_context_sampler_view(st, stObj);
+  }
}
 
st_context_free_zombie_objects(st);
@@ -933,11 +957,6 @@ st_destroy_context(struct st_context *st)
mtx_destroy(>zombie_sampler_views.mutex);
mtx_destroy(>zombie_shaders.mutex);
 
-   /* This must be called first so that glthread has a chance to finish */
-   _mesa_glthread_destroy(ctx);
-
-   _mesa_HashWalk(ctx->Shared->TexObjects, destroy_tex_sampler_cb, st);
-
st_reference_fragprog(st, >fp, NULL);
st_reference_prog(st, >gp, NULL);
st_reference_vertprog(st, >vp, NULL);
@@ -965,4 +984,7 @@ st_destroy_context(struct st_context *st)
st = NULL;
 
free(ctx);
+
+   /* Restore the current context and draw/read buffers */
+   _mesa_make_current(save_ctx, save_drawbuffer, save_readbuffer);
 }
-- 
1.8.5.6

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

Re: [Mesa-dev] [PATCH 1/2] softpipe: fix 32-bit bitfield extract

[Brian Paul]

All four patches look good to me

Reviewed-by: Brian Paul 

On 03/20/2019 10:16 PM, Dave Airlie wrote:

From: Dave Airlie 

These didn't deal with the width == 32 case that TGSI is defined with.

Fixes piglit tests if ARB_gpu_shader5 is enabled.
---
  src/gallium/auxiliary/tgsi/tgsi_exec.c | 14 --
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c 
b/src/gallium/auxiliary/tgsi/tgsi_exec.c
index e1181aa1932..c93e4e26e40 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
@@ -4944,8 +4944,13 @@ micro_ibfe(union tgsi_exec_channel *dst,
  {
 int i;
 for (i = 0; i < 4; i++) {
-  int width = src2->i[i] & 0x1f;
+  int width = src2->i[i];
int offset = src1->i[i] & 0x1f;
+  if (width == 32 && offset == 0) {
+ dst->i[i] = src0->i[i];
+ continue;
+  }
+  width &= 0x1f;
if (width == 0)
   dst->i[i] = 0;
else if (width + offset < 32)
@@ -4966,8 +4971,13 @@ micro_ubfe(union tgsi_exec_channel *dst,
  {
 int i;
 for (i = 0; i < 4; i++) {
-  int width = src2->u[i] & 0x1f;
+  int width = src2->u[i];
int offset = src1->u[i] & 0x1f;
+  if (width == 32 && offset == 0) {
+ dst->u[i] = src0->u[i];
+ continue;
+  }
+  width &= 0x1f;
if (width == 0)
   dst->u[i] = 0;
else if (width + offset < 32)



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

Re: [Mesa-dev] [PATCH] softpipe: fix texture view crashes

[Brian Paul]

On 03/19/2019 09:13 PM, Dave Airlie wrote:

From: Dave Airlie 

I noticed we crashed piglit arb_texture_view-rendering-formats
when run on softpipe.

This fixes the clear tiles to use the surface format not the
underlying storage format.

This fixes a bunch of srgb piglits as well.
---
  src/gallium/drivers/softpipe/sp_tile_cache.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/softpipe/sp_tile_cache.c 
b/src/gallium/drivers/softpipe/sp_tile_cache.c
index 351736ee421..998939bdf30 100644
--- a/src/gallium/drivers/softpipe/sp_tile_cache.c
+++ b/src/gallium/drivers/softpipe/sp_tile_cache.c
@@ -373,17 +373,18 @@ sp_tile_cache_flush_clear(struct softpipe_tile_cache *tc, 
int layer)
 if (util_format_is_pure_uint(tc->surface->format)) {
pipe_put_tile_ui_format(pt, tc->transfer_map[layer],
x, y, TILE_SIZE, TILE_SIZE,
-  pt->resource->format,
+  tc->surface->format,
(unsigned *) 
tc->tile->data.colorui128);
 } else if (util_format_is_pure_sint(tc->surface->format)) {
pipe_put_tile_i_format(pt, tc->transfer_map[layer],
   x, y, TILE_SIZE, TILE_SIZE,
- pt->resource->format,
+ tc->surface->format,
   (int *) tc->tile->data.colori128);
 } else {
-  pipe_put_tile_rgba(pt, tc->transfer_map[layer],
- x, y, TILE_SIZE, TILE_SIZE,
- (float *) tc->tile->data.color);
+  pipe_put_tile_rgba_format(pt, tc->transfer_map[layer],
+x, y, TILE_SIZE, TILE_SIZE,
+tc->surface->format,
+(float *) tc->tile->data.color);
     }
  }
  numCleared++;



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

[Mesa-dev] [PATCH] st/mesa: implement "zombie" sampler views (v2)

[Brian Paul]

When st_texture_release_all_sampler_views() is called the texture may
have sampler views belonging to several contexts.  If we unreference a
sampler view and its refcount hits zero, we need to be sure to destroy
the sampler view with the same context which created it.

This was not the case with the previous code which used
pipe_sampler_view_release().  That function could end up freeing a
sampler view with a context different than the one which created it.
In the case of the VMware svga driver, we detected this but leaked the
sampler view.  This led to a crash with google-chrome when the kernel
module had too many sampler views.  VMware bug 2274734.

Alternately, if we try to delete a sampler view with the correct
context, we may be "reaching into" a context which is active on
another thread.  That's not safe.

To fix these issues this patch adds a per-context list of "zombie"
sampler views.  These are views which are to be freed at some point
when the context is active.  Other contexts may safely add sampler
views to the zombie list at any time (it's mutex protected).  This
avoids the context/view ownership mix-ups we had before.

Tested with: google-chrome, google earth, Redway3D Watch/Turbine demos
a few Linux games.  If anyone can recomment some other multi-threaded,
multi-context GL apps to test, please let me know.

v2: avoid potential race issue by always adding sampler views to the
zombie list if the view's context doesn't match the current context,
ignoring the refcount.

Reviewed-by: Roland Scheidegger 
Reviewed-by: Neha Bhende 
Reviewed-by: Mathias Fröhlich 
Reviewed-By: Jose Fonseca 
---
 src/mesa/state_tracker/st_cb_flush.c |  6 +++
 src/mesa/state_tracker/st_context.c  | 80 
 src/mesa/state_tracker/st_context.h  | 25 ++
 src/mesa/state_tracker/st_sampler_view.c | 21 +++--
 src/mesa/state_tracker/st_texture.h  |  3 ++
 5 files changed, 131 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_flush.c 
b/src/mesa/state_tracker/st_cb_flush.c
index 5b3188c..81e5338 100644
--- a/src/mesa/state_tracker/st_cb_flush.c
+++ b/src/mesa/state_tracker/st_cb_flush.c
@@ -39,6 +39,7 @@
 #include "st_cb_flush.h"
 #include "st_cb_clear.h"
 #include "st_cb_fbo.h"
+#include "st_context.h"
 #include "st_manager.h"
 #include "pipe/p_context.h"
 #include "pipe/p_defines.h"
@@ -53,6 +54,11 @@ st_flush(struct st_context *st,
 {
st_flush_bitmap_cache(st);
 
+   /* We want to call this function periodically.
+* Typically, it has nothing to do so it shouldn't be expensive.
+*/
+   st_context_free_zombie_objects(st);
+
st->pipe->flush(st->pipe, fence, flags);
 }
 
diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index 2898279..c38f8e5 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -261,6 +261,79 @@ st_invalidate_state(struct gl_context *ctx)
 }
 
 
+/*
+ * In some circumstances (such as running google-chrome) the state
+ * tracker may try to delete a resource view from a context different
+ * than when it was created.  We don't want to do that.
+ *
+ * In that situation, st_texture_release_all_sampler_views() calls this
+ * function to transfer the sampler view reference to this context (expected
+ * to be the context which created the view.)
+ */
+void
+st_save_zombie_sampler_view(struct st_context *st,
+struct pipe_sampler_view *view)
+{
+   struct st_zombie_sampler_view_node *entry;
+
+   assert(view->context == st->pipe);
+
+   entry = MALLOC_STRUCT(st_zombie_sampler_view_node);
+   if (!entry)
+  return;
+
+   entry->view = view;
+
+   /* We need a mutex since this function may be called from one thread
+* while free_zombie_resource_views() is called from another.
+*/
+   mtx_lock(>zombie_sampler_views.mutex);
+   LIST_ADDTAIL(>node, >zombie_sampler_views.list.node);
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * Free any zombie sampler views that may be attached to this context.
+ */
+static void
+free_zombie_sampler_views(struct st_context *st)
+{
+   struct st_zombie_sampler_view_node *entry, *next;
+
+   if (LIST_IS_EMPTY(>zombie_sampler_views.list.node)) {
+  return;
+   }
+
+   mtx_lock(>zombie_sampler_views.mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next,
+>zombie_sampler_views.list.node, node) {
+  LIST_DEL(>node);  // remove this entry from the list
+
+  assert(entry->view->context == st->pipe);
+  pipe_sampler_view_reference(>view, NULL);
+
+  free(entry);
+   }
+
+   assert(LIST_IS_EMPTY(>zombie_sampler_views.list.node));
+
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * This function is called periodically to free any zombie objects
+ * which are attached to this context.
+ */
+void
+st_context_free_zombie_objects(struct st_context *st)
+{
+   free_zombie_sampler_views(st);
+}
+
+
 static void
 

Re: [Mesa-dev] [PATCH 1/8] st/mesa: implement "zombie" sampler views

[Brian Paul]

On 03/15/2019 09:54 AM, Jose Fonseca wrote:

On 14/03/2019 19:37, Brian Paul wrote:

When st_texture_release_all_sampler_views() is called the texture may
have sampler views belonging to several contexts.  If we unreference a
sampler view and its refcount hits zero, we need to be sure to destroy
the sampler view with the same context which created it.

This was not the case with the previous code which used
pipe_sampler_view_release().  That function could end up freeing a
sampler view with a context different than the one which created it.
In the case of the VMware svga driver, we detected this but leaked the
sampler view.  This led to a crash with google-chrome when the kernel
module had too many sampler views.  VMware bug 2274734.

Alternately, if we try to delete a sampler view with the correct
context, we may be "reaching into" a context which is active on
another thread.  That's not safe.

To fix these issues this patch adds a per-context list of "zombie"
sampler views.  These are views which are to be freed at some point
when the context is active.  Other contexts may safely add sampler
views to the zombie list at any time (it's mutex protected).  This
avoids the context/view ownership mix-ups we had before.

Tested with: google-chrome, google earth, Redway3D Watch/Turbine demos
a few Linux games.  If anyone can recomment some other multi-threaded,
multi-context GL apps to test, please let me know.
---
  src/mesa/state_tracker/st_cb_flush.c |  6 +++
  src/mesa/state_tracker/st_context.c  | 81 


  src/mesa/state_tracker/st_context.h  | 25 ++
  src/mesa/state_tracker/st_sampler_view.c | 27 +--
  src/mesa/state_tracker/st_texture.h  |  3 ++
  5 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_flush.c 
b/src/mesa/state_tracker/st_cb_flush.c

index 5b3188c..81e5338 100644
--- a/src/mesa/state_tracker/st_cb_flush.c
+++ b/src/mesa/state_tracker/st_cb_flush.c
@@ -39,6 +39,7 @@
  #include "st_cb_flush.h"
  #include "st_cb_clear.h"
  #include "st_cb_fbo.h"
+#include "st_context.h"
  #include "st_manager.h"
  #include "pipe/p_context.h"
  #include "pipe/p_defines.h"
@@ -53,6 +54,11 @@ st_flush(struct st_context *st,
  {
 st_flush_bitmap_cache(st);
+   /* We want to call this function periodically.
+    * Typically, it has nothing to do so it shouldn't be expensive.
+    */
+   st_context_free_zombie_objects(st);
+
 st->pipe->flush(st->pipe, fence, flags);
  }
diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c

index 2898279..bd919da 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -261,6 +261,80 @@ st_invalidate_state(struct gl_context *ctx)
  }
+/*
+ * In some circumstances (such as running google-chrome) the state
+ * tracker may try to delete a resource view from a context different
+ * than when it was created.  We don't want to do that.
+ * In that situation, st_texture_release_all_sampler_views() calls this
+ * function to save the view for later deletion.  The context here is
+ * expected to be the context which created the view.
+ */
+void
+st_save_zombie_sampler_view(struct st_context *st,
+    struct pipe_sampler_view *view)
+{
+   struct st_zombie_sampler_view_node *entry;
+
+   assert(view->context == st->pipe);
+   assert(view->reference.count == 1);
+
+   entry = MALLOC_STRUCT(st_zombie_sampler_view_node);
+   if (!entry)
+  return;
+
+   entry->view = view;
+
+   /* We need a mutex since this function may be called from one thread
+    * while free_zombie_resource_views() is called from another.
+    */
+   mtx_lock(>zombie_sampler_views.mutex);
+   LIST_ADDTAIL(>node, >zombie_sampler_views.list.node);
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * Free any zombie sampler views that may be attached to this context.
+ */
+static void
+free_zombie_sampler_views(struct st_context *st)
+{
+   struct st_zombie_sampler_view_node *entry, *next;
+
+   if (LIST_IS_EMPTY(>zombie_sampler_views.list.node)) {
+  return;
+   }
+
+   mtx_lock(>zombie_sampler_views.mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next,
+    >zombie_sampler_views.list.node, node) {
+  LIST_DEL(>node);  // remove this entry from the list
+
+  assert(entry->view->context == st->pipe);
+  assert(entry->view->reference.count == 1);
+  pipe_sampler_view_reference(>view, NULL);
+
+  free(entry);
+   }
+
+   assert(LIST_IS_EMPTY(>zombie_sampler_views.list.node));
+
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * This function is called periodically to free any zombie objects
+ * which are attached to this context.
+ */
+void
+st_context_free_zombie_objects(struct st_c

[Mesa-dev] [PATCH 2/8] st/mesa: implement "zombie" shaders list

[Brian Paul]

As with the preceding patch for sampler views, this patch does
basically the same thing but for shaders.  However, reference counting
isn't needed here (instead of calling cso_delete_XXX_shader() we call
st_save_zombie_shader().

The Redway3D Watch is one app/demo that needs this change.  Otherwise,
the vmwgfx driver generates an error about trying to destroy a shader
ID that doesn't exist in the context.

Note that if PIPE_CAP_SHAREABLE_SHADERS = TRUE, then we can use/delete
any shader with any context and this mechanism is not used.

Tested with: google-chrome, google earth, Redway3D Watch/Turbine demos
and a few Linux games.
---
 src/mesa/state_tracker/st_context.c | 86 +
 src/mesa/state_tracker/st_context.h | 23 ++
 src/mesa/state_tracker/st_program.c | 77 -
 3 files changed, 166 insertions(+), 20 deletions(-)

diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index bd919da..1e5742b 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -294,6 +294,39 @@ st_save_zombie_sampler_view(struct st_context *st,
 
 
 /*
+ * Since OpenGL shaders may be shared among contexts, we can wind up
+ * with variants of a shader created with different contexts.
+ * When we go to destroy a gallium shader, we want to free it with the
+ * same context that it was created with, unless the driver reports
+ * PIPE_CAP_SHAREABLE_SHADERS = TRUE.
+ */
+void
+st_save_zombie_shader(struct st_context *st,
+  enum pipe_shader_type type,
+  struct pipe_shader_state *shader)
+{
+   struct st_zombie_shader_node *entry;
+
+   /* we shouldn't be here if the driver supports shareable shaders */
+   assert(!st->has_shareable_shaders);
+
+   entry = MALLOC_STRUCT(st_zombie_shader_node);
+   if (!entry)
+  return;
+
+   entry->shader = shader;
+   entry->type = type;
+
+   /* We need a mutex since this function may be called from one thread
+* while free_zombie_shaders() is called from another.
+*/
+   mtx_lock(>zombie_shaders.mutex);
+   LIST_ADDTAIL(>node, >zombie_shaders.list.node);
+   mtx_unlock(>zombie_shaders.mutex);
+}
+
+
+/*
  * Free any zombie sampler views that may be attached to this context.
  */
 static void
@@ -325,6 +358,55 @@ free_zombie_sampler_views(struct st_context *st)
 
 
 /*
+ * Free any zombie shaders that may be attached to this context.
+ */
+static void
+free_zombie_shaders(struct st_context *st)
+{
+   struct st_zombie_shader_node *entry, *next;
+
+   if (LIST_IS_EMPTY(>zombie_shaders.list.node)) {
+  return;
+   }
+
+   mtx_lock(>zombie_shaders.mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next,
+>zombie_shaders.list.node, node) {
+  LIST_DEL(>node);  // remove this entry from the list
+
+  switch (entry->type) {
+  case PIPE_SHADER_VERTEX:
+ cso_delete_vertex_shader(st->cso_context, entry->shader);
+ break;
+  case PIPE_SHADER_FRAGMENT:
+ cso_delete_fragment_shader(st->cso_context, entry->shader);
+ break;
+  case PIPE_SHADER_GEOMETRY:
+ cso_delete_geometry_shader(st->cso_context, entry->shader);
+ break;
+  case PIPE_SHADER_TESS_CTRL:
+ cso_delete_tessctrl_shader(st->cso_context, entry->shader);
+ break;
+  case PIPE_SHADER_TESS_EVAL:
+ cso_delete_tesseval_shader(st->cso_context, entry->shader);
+ break;
+  case PIPE_SHADER_COMPUTE:
+ cso_delete_compute_shader(st->cso_context, entry->shader);
+ break;
+  default:
+ unreachable("invalid shader type in free_zombie_shaders()");
+  }
+  free(entry);
+   }
+
+   assert(LIST_IS_EMPTY(>zombie_shaders.list.node));
+
+   mtx_unlock(>zombie_shaders.mutex);
+}
+
+
+/*
  * This function is called periodically to free any zombie objects
  * which are attached to this context.
  */
@@ -332,6 +414,7 @@ void
 st_context_free_zombie_objects(struct st_context *st)
 {
free_zombie_sampler_views(st);
+   free_zombie_shaders(st);
 }
 
 
@@ -644,6 +727,8 @@ st_create_context_priv(struct gl_context *ctx, struct 
pipe_context *pipe,
 
LIST_INITHEAD(>zombie_sampler_views.list.node);
mtx_init(>zombie_sampler_views.mutex, mtx_plain);
+   LIST_INITHEAD(>zombie_shaders.list.node);
+   mtx_init(>zombie_shaders.mutex, mtx_plain);
 
return st;
 }
@@ -848,6 +933,7 @@ st_destroy_context(struct st_context *st)
st_context_free_zombie_objects(st);
 
mtx_destroy(>zombie_sampler_views.mutex);
+   mtx_destroy(>zombie_shaders.mutex);
 
/* This must be called first so that glthread has a chance to finish */
_mesa_glthread_destroy(ctx);
diff --git a/src/mesa/state_tracker/st_context.h 
b/src/mesa/state_tracker/st_context.h
index 1106bb6..c87ff81 100644
--- a/src/mesa/state_tracker/st_context.h
+++ b/src/mesa/state_tracker/st_context.h
@@ -106,6 +106,17 @@ struct st_zombie_sampler_view_node
 };

[Mesa-dev] [PATCH 6/8] swr: remove call to pipe_sampler_view_release()

[Brian Paul]

As with svga, llvmpipe drivers in previous patches.
Compile tested only.
---
 src/gallium/drivers/swr/swr_state.cpp | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index d7baa71..42d196f 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -302,11 +302,6 @@ swr_set_sampler_views(struct pipe_context *pipe,
/* set the new sampler views */
ctx->num_sampler_views[shader] = num;
for (i = 0; i < num; i++) {
-  /* Note: we're using pipe_sampler_view_release() here to work around
-   * a possible crash when the old view belongs to another context that
-   * was already destroyed.
-   */
-  pipe_sampler_view_release(pipe, >sampler_views[shader][start + i]);
   pipe_sampler_view_reference(>sampler_views[shader][start + i],
   views[i]);
}
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 5/8] llvmpipe: stop using pipe_sampler_view_release()

[Brian Paul]

This was used to avoid freeing a sampler view which was created by a
context that was already deleted.  But the state tracker does not
allow that.
---
 src/gallium/drivers/llvmpipe/lp_state_sampler.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_state_sampler.c 
b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
index c9aba1a..72823e4 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_sampler.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
@@ -123,12 +123,6 @@ llvmpipe_set_sampler_views(struct pipe_context *pipe,
 
/* set the new sampler views */
for (i = 0; i < num; i++) {
-  /* Note: we're using pipe_sampler_view_release() here to work around
-   * a possible crash when the old view belongs to another context that
-   * was already destroyed.
-   */
-  pipe_sampler_view_release(pipe,
->sampler_views[shader][start + i]);
   /*
* Warn if someone tries to set a view created in a different context
* (which is why we need the hack above in the first place).
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 3/8] st/mesa: stop using pipe_sampler_view_release()

[Brian Paul]

In all instances here we can replace pipe_sampler_view_release(pipe,
view) with pipe_sampler_view_reference(view, NULL) because the views
in question are private to the state tracker context.  So there's no
danger of freeing a sampler view with the wrong context.

Testing done: google chrome, misc GL demos, games
---
 src/mesa/state_tracker/st_context.c  | 3 +--
 src/mesa/state_tracker/st_sampler_view.c | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index 1e5742b..de98ddc 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -435,8 +435,7 @@ st_destroy_context_priv(struct st_context *st, bool 
destroy_pipe)
st_destroy_bound_image_handles(st);
 
for (i = 0; i < ARRAY_SIZE(st->state.frag_sampler_views); i++) {
-  pipe_sampler_view_release(st->pipe,
->state.frag_sampler_views[i]);
+  pipe_sampler_view_reference(>state.frag_sampler_views[i], NULL);
}
 
/* free glReadPixels cache data */
diff --git a/src/mesa/state_tracker/st_sampler_view.c 
b/src/mesa/state_tracker/st_sampler_view.c
index d16d523..23e6c6e 100644
--- a/src/mesa/state_tracker/st_sampler_view.c
+++ b/src/mesa/state_tracker/st_sampler_view.c
@@ -74,7 +74,7 @@ st_texture_set_sampler_view(struct st_context *st,
   if (sv->view) {
  /* check if the context matches */
  if (sv->view->context == st->pipe) {
-pipe_sampler_view_release(st->pipe, >view);
+pipe_sampler_view_reference(>view, NULL);
 goto found;
  }
   } else {
@@ -94,13 +94,13 @@ st_texture_set_sampler_view(struct st_context *st,
 
  if (new_max < views->max ||
  new_max > (UINT_MAX - sizeof(*views)) / sizeof(views->views[0])) {
-pipe_sampler_view_release(st->pipe, );
+pipe_sampler_view_reference(, NULL);
 goto out;
  }
 
  struct st_sampler_views *new_views = malloc(new_size);
  if (!new_views) {
-pipe_sampler_view_release(st->pipe, );
+pipe_sampler_view_reference(, NULL);
 goto out;
  }
 
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 1/8] st/mesa: implement "zombie" sampler views

[Brian Paul]

When st_texture_release_all_sampler_views() is called the texture may
have sampler views belonging to several contexts.  If we unreference a
sampler view and its refcount hits zero, we need to be sure to destroy
the sampler view with the same context which created it.

This was not the case with the previous code which used
pipe_sampler_view_release().  That function could end up freeing a
sampler view with a context different than the one which created it.
In the case of the VMware svga driver, we detected this but leaked the
sampler view.  This led to a crash with google-chrome when the kernel
module had too many sampler views.  VMware bug 2274734.

Alternately, if we try to delete a sampler view with the correct
context, we may be "reaching into" a context which is active on
another thread.  That's not safe.

To fix these issues this patch adds a per-context list of "zombie"
sampler views.  These are views which are to be freed at some point
when the context is active.  Other contexts may safely add sampler
views to the zombie list at any time (it's mutex protected).  This
avoids the context/view ownership mix-ups we had before.

Tested with: google-chrome, google earth, Redway3D Watch/Turbine demos
a few Linux games.  If anyone can recomment some other multi-threaded,
multi-context GL apps to test, please let me know.
---
 src/mesa/state_tracker/st_cb_flush.c |  6 +++
 src/mesa/state_tracker/st_context.c  | 81 
 src/mesa/state_tracker/st_context.h  | 25 ++
 src/mesa/state_tracker/st_sampler_view.c | 27 +--
 src/mesa/state_tracker/st_texture.h  |  3 ++
 5 files changed, 138 insertions(+), 4 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_flush.c 
b/src/mesa/state_tracker/st_cb_flush.c
index 5b3188c..81e5338 100644
--- a/src/mesa/state_tracker/st_cb_flush.c
+++ b/src/mesa/state_tracker/st_cb_flush.c
@@ -39,6 +39,7 @@
 #include "st_cb_flush.h"
 #include "st_cb_clear.h"
 #include "st_cb_fbo.h"
+#include "st_context.h"
 #include "st_manager.h"
 #include "pipe/p_context.h"
 #include "pipe/p_defines.h"
@@ -53,6 +54,11 @@ st_flush(struct st_context *st,
 {
st_flush_bitmap_cache(st);
 
+   /* We want to call this function periodically.
+* Typically, it has nothing to do so it shouldn't be expensive.
+*/
+   st_context_free_zombie_objects(st);
+
st->pipe->flush(st->pipe, fence, flags);
 }
 
diff --git a/src/mesa/state_tracker/st_context.c 
b/src/mesa/state_tracker/st_context.c
index 2898279..bd919da 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -261,6 +261,80 @@ st_invalidate_state(struct gl_context *ctx)
 }
 
 
+/*
+ * In some circumstances (such as running google-chrome) the state
+ * tracker may try to delete a resource view from a context different
+ * than when it was created.  We don't want to do that.
+ * In that situation, st_texture_release_all_sampler_views() calls this
+ * function to save the view for later deletion.  The context here is
+ * expected to be the context which created the view.
+ */
+void
+st_save_zombie_sampler_view(struct st_context *st,
+struct pipe_sampler_view *view)
+{
+   struct st_zombie_sampler_view_node *entry;
+
+   assert(view->context == st->pipe);
+   assert(view->reference.count == 1);
+
+   entry = MALLOC_STRUCT(st_zombie_sampler_view_node);
+   if (!entry)
+  return;
+
+   entry->view = view;
+
+   /* We need a mutex since this function may be called from one thread
+* while free_zombie_resource_views() is called from another.
+*/
+   mtx_lock(>zombie_sampler_views.mutex);
+   LIST_ADDTAIL(>node, >zombie_sampler_views.list.node);
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * Free any zombie sampler views that may be attached to this context.
+ */
+static void
+free_zombie_sampler_views(struct st_context *st)
+{
+   struct st_zombie_sampler_view_node *entry, *next;
+
+   if (LIST_IS_EMPTY(>zombie_sampler_views.list.node)) {
+  return;
+   }
+
+   mtx_lock(>zombie_sampler_views.mutex);
+
+   LIST_FOR_EACH_ENTRY_SAFE(entry, next,
+>zombie_sampler_views.list.node, node) {
+  LIST_DEL(>node);  // remove this entry from the list
+
+  assert(entry->view->context == st->pipe);
+  assert(entry->view->reference.count == 1);
+  pipe_sampler_view_reference(>view, NULL);
+
+  free(entry);
+   }
+
+   assert(LIST_IS_EMPTY(>zombie_sampler_views.list.node));
+
+   mtx_unlock(>zombie_sampler_views.mutex);
+}
+
+
+/*
+ * This function is called periodically to free any zombie objects
+ * which are attached to this context.
+ */
+void
+st_context_free_zombie_objects(struct st_context *st)
+{
+   free_zombie_sampler_views(st);
+}
+
+
 static void
 st_destroy_context_priv(struct st_context *st, bool destroy_pipe)
 {
@@ -568,6 +642,9 @@ st_create_context_priv(struct gl_context *ctx, struct 
pipe_context *pipe,
/* Initialize context's winsys buffers 

[Mesa-dev] [PATCH 8/8] gallium/util: remove pipe_sampler_view_release()

[Brian Paul]

It's no longer used.
---
 src/gallium/auxiliary/util/u_inlines.h | 20 
 1 file changed, 20 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_inlines.h 
b/src/gallium/auxiliary/util/u_inlines.h
index fa1e920..567d3d0 100644
--- a/src/gallium/auxiliary/util/u_inlines.h
+++ b/src/gallium/auxiliary/util/u_inlines.h
@@ -192,26 +192,6 @@ pipe_sampler_view_reference(struct pipe_sampler_view **dst,
*dst = src;
 }
 
-/**
- * Similar to pipe_sampler_view_reference() but always set the pointer to
- * NULL and pass in the current context explicitly.
- *
- * If *ptr is non-NULL, it may refer to a view that was created in a different
- * context (however, that context must still be alive).
- */
-static inline void
-pipe_sampler_view_release(struct pipe_context *ctx,
-  struct pipe_sampler_view **ptr)
-{
-   struct pipe_sampler_view *old_view = *ptr;
-
-   if (pipe_reference_described(_view->reference, NULL,
-(debug_reference_descriptor)debug_describe_sampler_view)) {
-  ctx->sampler_view_destroy(ctx, old_view);
-   }
-   *ptr = NULL;
-}
-
 static inline void
 pipe_so_target_reference(struct pipe_stream_output_target **dst,
  struct pipe_stream_output_target *src)
-- 
1.8.5.6

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

[Mesa-dev] [PATCH 4/8] svga: stop using pipe_sampler_view_release()

[Brian Paul]

This function was used in the past to avoid deleting a sampler view
for a context that no longer exists.  But the Mesa state tracker
ensures that cannot happen.  Use the standard refcounting function
instead.

Also, remove the code which checked for context mis-matches in
svga_sampler_view_destroy().  It's no longer needed since implementing
the zombie sampler view code in the state tracker.

Testing Done: google chrome, variety of GL demos/games
---
 src/gallium/drivers/svga/svga_pipe_sampler.c | 38 +---
 src/gallium/drivers/svga/svga_state_tss.c|  4 +--
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_pipe_sampler.c 
b/src/gallium/drivers/svga/svga_pipe_sampler.c
index b32bb09..f1e68dd 100644
--- a/src/gallium/drivers/svga/svga_pipe_sampler.c
+++ b/src/gallium/drivers/svga/svga_pipe_sampler.c
@@ -405,28 +405,18 @@ svga_sampler_view_destroy(struct pipe_context *pipe,
struct svga_pipe_sampler_view *sv = svga_pipe_sampler_view(view);
 
if (svga_have_vgpu10(svga) && sv->id != SVGA3D_INVALID_ID) {
-  if (view->context != pipe) {
- /* The SVGA3D device will generate an error (and on Linux, cause
-  * us to abort) if we try to destroy a shader resource view from
-  * a context other than the one it was created with.  Skip the
-  * SVGA3D_vgpu10_DestroyShaderResourceView() and leak the sampler
-  * view for now.  This should only sometimes happen when a shared
-  * texture is deleted.
-  */
- _debug_printf("context mismatch in %s\n", __func__);
-  }
-  else {
- enum pipe_error ret;
+  enum pipe_error ret;
+
+  assert(view->context == pipe);
 
- svga_hwtnl_flush_retry(svga); /* XXX is this needed? */
+  svga_hwtnl_flush_retry(svga);
 
+  ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
+  if (ret != PIPE_OK) {
+ svga_context_flush(svga, NULL);
  ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
- if (ret != PIPE_OK) {
-svga_context_flush(svga, NULL);
-ret = SVGA3D_vgpu10_DestroyShaderResourceView(svga->swc, sv->id);
- }
- util_bitmask_clear(svga->sampler_view_id_bm, sv->id);
   }
+  util_bitmask_clear(svga->sampler_view_id_bm, sv->id);
}
 
pipe_resource_reference(>base.texture, NULL);
@@ -465,7 +455,8 @@ svga_set_sampler_views(struct pipe_context *pipe,
 */
if (start == 0 && num == 0 && svga->curr.num_sampler_views[shader] > 0) {
   for (i = 0; i < svga->curr.num_sampler_views[shader]; i++) {
- pipe_sampler_view_release(pipe, >curr.sampler_views[shader][i]);
+ pipe_sampler_view_reference(>curr.sampler_views[shader][i],
+ NULL);
   }
   any_change = TRUE;
}
@@ -474,11 +465,6 @@ svga_set_sampler_views(struct pipe_context *pipe,
   enum pipe_texture_target target;
 
   if (svga->curr.sampler_views[shader][start + i] != views[i]) {
- /* Note: we're using pipe_sampler_view_release() here to work around
-  * a possible crash when the old view belongs to another context that
-  * was already destroyed.
-  */
- pipe_sampler_view_release(pipe, 
>curr.sampler_views[shader][start + i]);
  pipe_sampler_view_reference(>curr.sampler_views[shader][start + 
i],
  views[i]);
  any_change = TRUE;
@@ -552,8 +538,8 @@ svga_cleanup_sampler_state(struct svga_context *svga)
   unsigned i;
 
   for (i = 0; i < svga->state.hw_draw.num_sampler_views[shader]; i++) {
- pipe_sampler_view_release(>pipe,
-   
>state.hw_draw.sampler_views[shader][i]);
+ 
pipe_sampler_view_reference(>state.hw_draw.sampler_views[shader][i],
+ NULL);
   }
}

diff --git a/src/gallium/drivers/svga/svga_state_tss.c 
b/src/gallium/drivers/svga/svga_state_tss.c
index d598d23..95b1a9e 100644
--- a/src/gallium/drivers/svga/svga_state_tss.c
+++ b/src/gallium/drivers/svga/svga_state_tss.c
@@ -50,8 +50,8 @@ svga_cleanup_tss_binding(struct svga_context *svga)
   struct svga_hw_view_state *view = >state.hw_draw.views[i];
   if (view) {
  svga_sampler_view_reference(>v, NULL);
- pipe_sampler_view_release(>pipe,
-   >curr.sampler_views[shader][i]);
+ pipe_sampler_view_reference(>curr.sampler_views[shader][i],
+ NULL);
  pipe_resource_reference(>texture, NULL);
  view->dirty = TRUE;
   }
-- 
1.8.5.6

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