[Mesa-dev] [PATCH] freedreno: fix slice pitch calculations
For example if width were 65, the first slice would get 96 while the
second would get 32. However the hardware appears to expect the second
pitch to be 64, based on halving the 96 (and aligning up to 32).
This fixes texelFetch piglit tests on a3xx.
Signed-off-by: Ilia Mirkin
Cc: "10.4 10.5"
---
Still need to run this through piglit, but the initial texelFetch
results are encouraging. There still seems to be some horrible problem
with array textures e.g. sampler2DArray 100x100x5, but it actually
feels related to rendering rather than sampling.
src/gallium/drivers/freedreno/freedreno_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/freedreno/freedreno_resource.c
b/src/gallium/drivers/freedreno/freedreno_resource.c
index afb396e..a49d5f5 100644
--- a/src/gallium/drivers/freedreno/freedreno_resource.c
+++ b/src/gallium/drivers/freedreno/freedreno_resource.c
@@ -213,7 +213,7 @@ setup_slices(struct fd_resource *rsc, uint32_t alignment)
for (level = 0; level <= prsc->last_level; level++) {
struct fd_resource_slice *slice = fd_resource_slice(rsc, level);
- slice->pitch = align(width, 32);
+ slice->pitch = width = align(width, 32);
slice->offset = size;
if (level == 0 || layers_in_level == 1)
slice->size0 = align(slice->pitch * height * rsc->cpp,
alignment);
--
2.0.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Summer of Code ideas (maybe just an idea wishlist?)
On Fri, Mar 13, 2015 at 12:46 AM, Matt Turner wrote: > Here are some ideas I think might be reasonable GSoC ideas. > > - GLSL linking in NIR > - Would allow us to stop doing optimizations and other expensive > things on GLSL IR > > - SSA in the i965/fs backend, and an SSA-based register allocator > > - Improve instruction scheduling in i965 (Nouveau has this on the > Wiki as well. Maybe potential for code reuse) FWIW the nouveau thing would probably not involve a lot of reuse... the instructions are fairly different, and a lot of the nouveau project would have to be actually figuring out what _is_ a better schedule rather than just implementing one. [Also nouveau presently has *no* scheduling in place... so almost _anything_ would likely be an improvement.] > > - Maybe some OpenGL features (???), although lots of things are > already in progress, like tessellation and compute shaders and others > are probably on the critical path > > Are these good GSoC projects? Good -- being a combination of > characteristics like usefulness, feasibility, well defined, and > properly sized. > > Feel free to comment on them and to reply with your own. We can pick > some we like and put them on the wiki. Another project that I think I mentioned on IRC is to go through the ES extension list and add in all the "easy" ones (i.e. ones that are already all or mostly implemented by desktop features), of which there are a _ton_ (probably like 20). I'm just talking about the OES/EXT/KHR ones, not the vendor ones. The majority would be modifying the various paths to allow the logic to be executed in ES contexts and to modify/add piglit tests verifying that the functionality can be used in ES. Here are a bunch of differences between what the nvidia blob exposes and mesa for the same hw: http://people.freedesktop.org/~imirkin/glxinfo/glxinfo.html#p=es&b=version&g=NVIDIA%20GF1xx%20%28GeForce%20400%2C%20500%29&diff Admittedly some of those are ES3.1-only, but far from all. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
On Thu, Mar 12, 2015 at 9:24 PM, Jason Ekstrand
wrote:
> Sean,
> I got ready to push this and ran it against piglit and one of the tests
> errored out. Looking into it further, it was a bug in the test. I've sent
> a patch:
>
> http://lists.freedesktop.org/archives/piglit/2015-March/015156.html
>
> I'll merge the mesa commit once we get piglit fixed.
> --Jason
>
Pushed. Thanks!
>
> On Thu, Mar 12, 2015 at 4:38 PM, Sean Burke wrote:
>
>> Jason,
>>
>> No worries. It looks like my mail client munged the patch in some way.
>> I'm sending it as an attachment in the hopes that it will remain
>> untouched.
>>
>>
>> Sean Burke
>>
>> On Wed, Mar 11, 2015 at 2:53 PM, Jason Ekstrand
>> wrote:
>> > Sean,
>> > Sorry it's taken so long for me to get to this, but I went to test/push
>> this
>> > today and it doesn't apply against current mesa master. Can you please
>> > either rebase on master and re-send or give me the SHA1 hash of the
>> commit
>> > this one is based on. (Not the SHA1 of this commit but the previous
>> one).
>> > --Jason
>> >
>> > On Sat, Mar 7, 2015 at 8:34 PM, Sean Burke
>> wrote:
>> >>
>> >> The memory layout of compatible internal formats may differ in bytes
>> per
>> >> block, so TexFormat is not a reliable measure of compatibility. For
>> >> example,
>> >> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid
>> out
>> >> in
>> >> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout,
>> the
>> >> existing compatibility check will fail.
>> >>
>> >> Additionally, the current check allows any two compressed textures
>> which
>> >> share
>> >> block size to be used, whereas the spec gives an explicit table of
>> >> compatible
>> >> formats.
>> >>
>> >> v2: Use a switch instead of array iteration for block class and show
>> the
>> >> correct GL error when internal formats are mismatched.
>> >> v3: Include spec citations for new compatibility checks, rearrange
>> check
>> >> order to ensure that compressed, view-compatible formats return the
>> >> correct result, and make style fixes. Original commit message
>> amended
>> >> for clarity.
>> >> v4: Reformatted spec citations.
>> >>
>> >> Reviewed-by: Jason Ekstrand
>> >> ---
>> >> src/mesa/main/copyimage.c | 151
>> >> +++---
>> >> 1 file changed, 130 insertions(+), 21 deletions(-)
>> >>
>> >> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
>> >> index 455929d..fd22f28 100644
>> >> --- a/src/mesa/main/copyimage.c
>> >> +++ b/src/mesa/main/copyimage.c
>> >> @@ -33,6 +33,12 @@
>> >> #include "texobj.h"
>> >> #include "fbobject.h"
>> >> #include "textureview.h"
>> >> +#include "glformats.h"
>> >> +
>> >> +enum mesa_block_class {
>> >> + BLOCK_CLASS_128_BITS,
>> >> + BLOCK_CLASS_64_BITS
>> >> +};
>> >>
>> >> static bool
>> >> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target,
>> int
>> >> level,
>> >> @@ -253,6 +259,124 @@ check_region_bounds(struct gl_context *ctx,
>> >> struct gl_texture_image *tex_image,
>> >> return true;
>> >> }
>> >>
>> >> +static bool
>> >> +compressed_format_compatible(struct gl_context *ctx,
>> >> + GLenum compressedFormat, GLenum
>> otherFormat)
>> >> +{
>> >> + enum mesa_block_class compressedClass, otherClass;
>> >> +
>> >> + /* Two view-incompatible compressed formats are never compatible.
>> */
>> >> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
>> >> + return false;
>> >> + }
>> >> +
>> >> + /*
>> >> +* From ARB_copy_image spec:
>> >> +*Table 4.X.1 (Compatible internal formats for copying between
>> >> +* compressed and uncompressed internal formats)
>> >> +*
>> >> -
>> >> +*| Texel / | Uncompressed |
>> >> |
>> >> +*| Block | internal format | Compressed internal format
>> >> |
>> >> +*| size| |
>> >> |
>> >> +*
>> >> -
>> >> +*| 128-bit | RGBA32UI, |
>> COMPRESSED_RGBA_S3TC_DXT3_EXT,
>> >> |
>> >> +*| | RGBA32I, |
>> >> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
>> >> +*| | RGBA32F |
>> COMPRESSED_RGBA_S3TC_DXT5_EXT,
>> >> |
>> >> +*| | |
>> >> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
>> >> +*| | | COMPRESSED_RG_RGTC2,
>> >> |
>> >> +*| | | COMPRESSED_SIGNED_RG_RGTC2,
>> >> |
>> >> +*| | | COMPRESSED_RGBA_BPTC_UNORM,
>> >> |
>> >> +*| | |
>> >> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
>> >> +*| | |
>> >> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
>> >> +*| | |
>> >> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
>> >> +
[Mesa-dev] [PATCH] freedreno/a3xx: use the same layer size for all slices
We only program in one layer size per texture, so that means that all levels must share one size. This makes the piglit test bin/texelFetch fs sampler2DArray have the same breakage as its non-array version instead of being completely off. Signed-off-by: Ilia Mirkin Cc: "10.4 10.5" --- Need to put this through a piglit run, but seems fairly obvious given what fd3_texture does. Also see results of the texelFetch piglit. src/gallium/drivers/freedreno/freedreno_resource.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/freedreno/freedreno_resource.c b/src/gallium/drivers/freedreno/freedreno_resource.c index c412407..afb396e 100644 --- a/src/gallium/drivers/freedreno/freedreno_resource.c +++ b/src/gallium/drivers/freedreno/freedreno_resource.c @@ -215,7 +215,10 @@ setup_slices(struct fd_resource *rsc, uint32_t alignment) slice->pitch = align(width, 32); slice->offset = size; - slice->size0 = align(slice->pitch * height * rsc->cpp, alignment); + if (level == 0 || layers_in_level == 1) + slice->size0 = align(slice->pitch * height * rsc->cpp, alignment); + else + slice->size0 = rsc->slices[0].size0; size += slice->size0 * depth * layers_in_level; -- 2.0.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Summer of Code ideas (maybe just an idea wishlist?)
Here are some ideas I think might be reasonable GSoC ideas. - GLSL linking in NIR - Would allow us to stop doing optimizations and other expensive things on GLSL IR - SSA in the i965/fs backend, and an SSA-based register allocator - Improve instruction scheduling in i965 (Nouveau has this on the Wiki as well. Maybe potential for code reuse) - Maybe some OpenGL features (???), although lots of things are already in progress, like tessellation and compute shaders and others are probably on the critical path Are these good GSoC projects? Good -- being a combination of characteristics like usefulness, feasibility, well defined, and properly sized. Feel free to comment on them and to reply with your own. We can pick some we like and put them on the wiki. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
Sean,
I got ready to push this and ran it against piglit and one of the tests
errored out. Looking into it further, it was a bug in the test. I've sent
a patch:
http://lists.freedesktop.org/archives/piglit/2015-March/015156.html
I'll merge the mesa commit once we get piglit fixed.
--Jason
On Thu, Mar 12, 2015 at 4:38 PM, Sean Burke wrote:
> Jason,
>
> No worries. It looks like my mail client munged the patch in some way.
> I'm sending it as an attachment in the hopes that it will remain
> untouched.
>
>
> Sean Burke
>
> On Wed, Mar 11, 2015 at 2:53 PM, Jason Ekstrand
> wrote:
> > Sean,
> > Sorry it's taken so long for me to get to this, but I went to test/push
> this
> > today and it doesn't apply against current mesa master. Can you please
> > either rebase on master and re-send or give me the SHA1 hash of the
> commit
> > this one is based on. (Not the SHA1 of this commit but the previous
> one).
> > --Jason
> >
> > On Sat, Mar 7, 2015 at 8:34 PM, Sean Burke
> wrote:
> >>
> >> The memory layout of compatible internal formats may differ in bytes per
> >> block, so TexFormat is not a reliable measure of compatibility. For
> >> example,
> >> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid
> out
> >> in
> >> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout,
> the
> >> existing compatibility check will fail.
> >>
> >> Additionally, the current check allows any two compressed textures which
> >> share
> >> block size to be used, whereas the spec gives an explicit table of
> >> compatible
> >> formats.
> >>
> >> v2: Use a switch instead of array iteration for block class and show the
> >> correct GL error when internal formats are mismatched.
> >> v3: Include spec citations for new compatibility checks, rearrange check
> >> order to ensure that compressed, view-compatible formats return the
> >> correct result, and make style fixes. Original commit message
> amended
> >> for clarity.
> >> v4: Reformatted spec citations.
> >>
> >> Reviewed-by: Jason Ekstrand
> >> ---
> >> src/mesa/main/copyimage.c | 151
> >> +++---
> >> 1 file changed, 130 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> >> index 455929d..fd22f28 100644
> >> --- a/src/mesa/main/copyimage.c
> >> +++ b/src/mesa/main/copyimage.c
> >> @@ -33,6 +33,12 @@
> >> #include "texobj.h"
> >> #include "fbobject.h"
> >> #include "textureview.h"
> >> +#include "glformats.h"
> >> +
> >> +enum mesa_block_class {
> >> + BLOCK_CLASS_128_BITS,
> >> + BLOCK_CLASS_64_BITS
> >> +};
> >>
> >> static bool
> >> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
> >> level,
> >> @@ -253,6 +259,124 @@ check_region_bounds(struct gl_context *ctx,
> >> struct gl_texture_image *tex_image,
> >> return true;
> >> }
> >>
> >> +static bool
> >> +compressed_format_compatible(struct gl_context *ctx,
> >> + GLenum compressedFormat, GLenum
> otherFormat)
> >> +{
> >> + enum mesa_block_class compressedClass, otherClass;
> >> +
> >> + /* Two view-incompatible compressed formats are never compatible. */
> >> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
> >> + return false;
> >> + }
> >> +
> >> + /*
> >> +* From ARB_copy_image spec:
> >> +*Table 4.X.1 (Compatible internal formats for copying between
> >> +* compressed and uncompressed internal formats)
> >> +*
> >> -
> >> +*| Texel / | Uncompressed |
> >> |
> >> +*| Block | internal format | Compressed internal format
> >> |
> >> +*| size| |
> >> |
> >> +*
> >> -
> >> +*| 128-bit | RGBA32UI, | COMPRESSED_RGBA_S3TC_DXT3_EXT,
> >> |
> >> +*| | RGBA32I, |
> >> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
> >> +*| | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT,
> >> |
> >> +*| | |
> >> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
> >> +*| | | COMPRESSED_RG_RGTC2,
> >> |
> >> +*| | | COMPRESSED_SIGNED_RG_RGTC2,
> >> |
> >> +*| | | COMPRESSED_RGBA_BPTC_UNORM,
> >> |
> >> +*| | |
> >> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
> >> +*| | |
> >> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
> >> +*| | |
> >> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
> >> +*
> >> -
> >> +*| 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT,
> >> |
> >> +*| | RGBA16UI, RG32UI, | COMPRESSED_SRGB_S3TC_DXT1_EXT,
> >> |
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
On 13.03.2015 03:07, Pierre-Loup A. Griffais wrote: > On 03/11/2015 09:40 AM, Ian Romanick wrote: >> On 03/11/2015 09:31 AM, Tobias Klausmann wrote: >>> The problem in not forcing this to link statically is, that if a >>> distribution decides to not use this static option, the problem persists >>> on that distribution. On top every lib pulled in by steam from the >>> system would need to be link statically, like mesa. Instead of fixing >>> every lib steam pulls in (how many are there?), fix the steam runtime to >> >> Yeah, static linking is a terrible, partial solution. >> >>> a) not ship libstdc++.so and libgcc_s.so and declare older version of >>> these libs as not supported (thats what people do when they face the >>> incompatibility problem with steams versions of these libs: just delete >>> them from the runtime and everything is fine) >> >> Let's be 100% clear. This is NOT an option for Steam. They ship >> thousands of closed-source applications. These applications were built >> and tested against specific versions of specific libraries. Removing >> support for old run-times is equivalent to removing support for those >> applications. They can't tell their customers, "You upgraded your >> distro, so that game you paid money for no longer works. Tough break, >> kid." > > Quite true; developers build and test their games against this known set > of libraries through the Steam Runtime toolchain. Expecting them to > manage all of their dependencies (through bundling or static linking) is > unreasonable; all other platforms they're used to dealing with solve > this problem one way or another. Admittedly the current implementation > of the Steam runtime is only a first step towards providing a solution, > and some things could be done better, but this issue will stand regardless. > > The C++ runtime is a very complex beast and swapping it under an > application that makes heavy use of it is prone to failure. We'll keep > running games against the C++ runtime of the SDK version they were built > against, forever. > > The LLVM-enabled variants of Mesa are the only implementations where > this is an issue; this is actually causing affected users in the wild to > switch to the Catalyst driver, since it (along with all others) goes out > of its way to avoid this specific problem for this exact reason. > > Their only other option is to slam the whole Steam runtime off, which > breaks a lot of games outright because they depend on some library that > recent distros don't ship anymore, or moved to the new major version. In my experience, and from other reports I've seen, simply deleting libstdc++ / libgcc from the Steam runtime works just fine. This shows that a) the problem is that Steam overrides the system versions of these libraries with older versions, b) doing that isn't necessary, as the games work just fine with the newer system versions of these libraries, since they preserve backwards compatibility. So, a simple solution would be for Steam not to override these libraries[0] when the system already has the same or newer versions of them available. That doesn't require deviating from long established best practices for building and distributing software in distros. [0] Note that "these libraries" refers to the specific SONAMEs, e.g. libstdc++.so.6 and libgcc_s.so.1. If the system only provides different SONAMEs for these libraries, it should be fine for Steam to use its own versions of these SONAMEs. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [GSoC] Improved application of GLSL complier optimizations
On Tue, Mar 3, 2015 at 11:43 AM, Alexander Mezin wrote: > Hello. > > I plan to participate in GSoC this year as a student. And I've found a > project idea which I like very much on X.Org's ideas page. It's called > "Improved application of GLSL compiler optimizations". > > For me, it looks mostly like an optimization problem in mathematical > meaning. Benchmark could be treated as a function that computes a > score taking a sequence of passes as input. If I'm not mistaken, there > are more than 30 passes, so even trying all permutations will take a > long time, even if we imagine we can measure shader "quality" > immediately. I think it's worth to try various heuristic optimization > techniques like simulated annealing or genetic algorithm. > > Furthermore, looks like the same pass sequence could give different > results on different shaders, so it won't be possible to determine > single optimal sequence. But some limited randomization could help > dealing with it, probably. > > Overall this project looks more like research than coding. I like it, > but I have few concerns: > 1. Probably the final result will look very stupid, like "do > everything twice in any order, everything else doesn't matter". Though > it will be proved at least. > 2. What to do next depends on results of the previous step. I'll > hardly be able to do very detailed plan. > > So, do you think it's worth trying? I don't think this project is very interesting, unfortunately. There are multiple sets of optimizations that run in loops until they collectively find some steady state: the GLSL compiler (see src/glsl/glsl_parser_extras.cpp::do_common_optimization), the NIR level for some drivers (see src/mesa/drivers/dri/i965/brw_fs_nir.cpp::nir_optimize), and the driver backend optimizations (see src/mesa/drivers/dri/i965/brw_fs.cpp::fs_visitor::optimize for example). Different orderings of the GLSL compiler's optimizations just leads to madness. Reordering causes all sorts of unexpected differences and really the IR is not good for optimizing to begin with. We want to stop doing optimizations there, really. NIR is a better IR for optimizing. There is maybe something to be learned by reordering optimizations there... I'm not sure. One thing that we do know already is that we don't have all of the optimizations in place that we want -- global value numbering is missing for instance. It shouldn't be hard to add, but no one's done it. Reordering optimizations in NIR probably isn't interesting at this point. Speaking for i965 here, since that's the driver I work on -- Kind of like NIR, we expect we can get much bigger gains by improving existing optimizations rather than figuring out a different ordering. We'd like to implement SSA in the i965/fs (struct-of-arrays) backend and an SSA-based register allocator. That might be a good summer of code project. But that's more coding than research (the research having been done ~10 years ago), so it might not be interesting to you. So really I don't think we want to improve the "application" of optimizations but rather simply improve the optimizations themselves. If you are interested in working on compiler optimizations, let us know. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12, 2015 at 3:53 PM, Carl Worth wrote: > On Thu, Mar 12 2015, Matt Turner wrote: >> I think you misread. rint() *does* provide the behavior we want >> (round-to-nearest, half to even) when the rounding mode is the default >> round-to-nearest. > > Thanks. I did at least verify that behaviorally as I just mentioned in a > separate mail. > >> As Eric noted in his original patch, the man pages for >> rint(3)/nearbyint(3) don't describe the half-to-even behavior but >> round(3) does: > > Ah. Yes, I was using the man pages, so that was the source of my > confusion. I did some more research. The draft C99 spec I have doesn't say anything about how rint/nearbyint should round halfway cases. Its description of the round() functions is explicit though: "rounding halfway cases away from zero". I think this is because it's not C that specifies how rint/nearbyint should round halfway cases -- it's IEEE-754 that says round to nearest (and halfway cases go to even). Maybe if you're on a VAX these functions round halfway cases differently. :) But round() is explicitly defined by the language itself to round halfway cases in a particular way, which explains the text in round(3). >> Maybe I should send a patch to the Linux man-pages project too. > > That definitely wouldn't hurt. Done! >>> assert (fegetround() == FE_TONEAREST); >> >> I don't really want to do this. We rely on the default rounding mode >> everywhere. I don't think asserting here is useful. > > What's useful is increasing the likelihood that when somebody does > violate the assumption, they are alterted to the issue, rather than > having Mesa quietly misbehave in hard-to-predict ways. > > It seems obvious to me that we should improve robustness when we've > already identified exactly where (at least one piece) of fragility lies > in Mesa. I don't really know that it is fragile. Though the GL spec doesn't say anything on the matter, I can't really imagine that libGL is expected to operate identically under different rounding modes. I certainly wouldn't expect that from other system libraries -- cairo for instance. This seems like an intractable problem short of saving and restoring the rounding mode in each GL entry point. (Not a real suggestion :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallivm: (trivial) Fix typo in comment introduced by 70dc8a
Fix typo in comment introduced by 70dc8a Signed-off-by: Alexandre Demers --- src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp index d60db91..4ede90b 100644 --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp @@ -502,7 +502,7 @@ lp_build_create_jit_compiler_for_module(LLVMExecutionEngineRef *OutJIT, #if HAVE_LLVM >= 0x0306 builder.setMCJITMemoryManager(std::unique_ptr(MM)); - MM = NULL; // onwership taken by std::unique_ptr + MM = NULL; // ownership taken by std::unique_ptr #else builder.setMCJITMemoryManager(MM); #endif -- 2.3.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
Am 12.03.2015 um 23:53 schrieb Carl Worth: > On Thu, Mar 12 2015, Matt Turner wrote: >> I think you misread. rint() *does* provide the behavior we want >> (round-to-nearest, half to even) when the rounding mode is the default >> round-to-nearest. > > Thanks. I did at least verify that behaviorally as I just mentioned in a > separate mail. > >> As Eric noted in his original patch, the man pages for >> rint(3)/nearbyint(3) don't describe the half-to-even behavior but >> round(3) does: > > Ah. Yes, I was using the man pages, so that was the source of my > confusion. > >> Maybe I should send a patch to the Linux man-pages project too. > > That definitely wouldn't hurt. > >>> assert (fegetround() == FE_TONEAREST); >> >> I don't really want to do this. We rely on the default rounding mode >> everywhere. I don't think asserting here is useful. > > What's useful is increasing the likelihood that when somebody does > violate the assumption, they are alterted to the issue, rather than > having Mesa quietly misbehave in hard-to-predict ways. > > It seems obvious to me that we should improve robustness when we've > already identified exactly where (at least one piece) of fragility lies > in Mesa. > >> Also it requires adding code for MSVC since it doesn't have >> fegetround(). > > Not code, no. I'd be happy with just the #ifndef to hide the call from > MSVC. I won't ask you find equivalent functionality for MSVC. > Honestly I'm not sure it makes all that much sense to have an assert here. Because imho such things should be reserved to detect internal inconsistencies, but obviously this is something mesa has absolutely no control of. The caller gets what it called for. I don't think other system libraries would assert if you'd call them with non-standard rounding modes. Plus app developers probably wouldn't notice anyway. Reporting through some debug mechanism would probably make more sense if anything, but just ignoring it is fine by me too - there's tons of other calculations done in mesa in other places which will return different results based on rounding modes. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mapi: Make private copies of name strings provided by client.
On 03/12/2015 04:34 PM, Mario Kleiner wrote:
glXGetProcAddress("glFoo") ends up in stub_add_dynamic() to
create dynamic stubs for dynamic functions. stub_add_dynamic()
doesn't store the caller provided name string "Foo" in a mesa
private copy, but just stores a pointer to the "glFoo" string
passed to glXGetProcAddress - a pointer into arbitrary memory
outside mesa's control.
If the caller passes some dynamically allocated/changing
memory buffer to glXGetProcAddress(), or the caller gets unmapped
from memory, e.g., some dynamically loaded application
plugin which uses OpenGL, this ends badly - with a dangling
pointer.
strdup() the name string provided by the client to avoid
this problem.
Cc: "10.3 10.4 10.5"
Signed-off-by: Mario Kleiner
---
src/mapi/stub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mapi/stub.c b/src/mapi/stub.c
index dfadbe1..45ccc6a 100644
--- a/src/mapi/stub.c
+++ b/src/mapi/stub.c
@@ -110,7 +110,7 @@ stub_add_dynamic(const char *name)
if (!stub->addr)
return NULL;
- stub->name = (const void *) name;
+ stub->name = (const void *) strdup(name);
/* to be fixed later */
stub->slot = -1;
LGTM.
Reviewed-by: Brian Paul
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4] mesa: improve ARB_copy_image internal format compat check
Jason,
No worries. It looks like my mail client munged the patch in some way.
I'm sending it as an attachment in the hopes that it will remain
untouched.
Sean Burke
On Wed, Mar 11, 2015 at 2:53 PM, Jason Ekstrand wrote:
> Sean,
> Sorry it's taken so long for me to get to this, but I went to test/push this
> today and it doesn't apply against current mesa master. Can you please
> either rebase on master and re-send or give me the SHA1 hash of the commit
> this one is based on. (Not the SHA1 of this commit but the previous one).
> --Jason
>
> On Sat, Mar 7, 2015 at 8:34 PM, Sean Burke wrote:
>>
>> The memory layout of compatible internal formats may differ in bytes per
>> block, so TexFormat is not a reliable measure of compatibility. For
>> example,
>> GL_RGB8 and GL_RGB8UI are compatible formats, but GL_RGB8 may be laid out
>> in
>> memory as B8G8R8X8. If GL_RGB8UI has a 3 byte-per-block memory layout, the
>> existing compatibility check will fail.
>>
>> Additionally, the current check allows any two compressed textures which
>> share
>> block size to be used, whereas the spec gives an explicit table of
>> compatible
>> formats.
>>
>> v2: Use a switch instead of array iteration for block class and show the
>> correct GL error when internal formats are mismatched.
>> v3: Include spec citations for new compatibility checks, rearrange check
>> order to ensure that compressed, view-compatible formats return the
>> correct result, and make style fixes. Original commit message amended
>> for clarity.
>> v4: Reformatted spec citations.
>>
>> Reviewed-by: Jason Ekstrand
>> ---
>> src/mesa/main/copyimage.c | 151
>> +++---
>> 1 file changed, 130 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
>> index 455929d..fd22f28 100644
>> --- a/src/mesa/main/copyimage.c
>> +++ b/src/mesa/main/copyimage.c
>> @@ -33,6 +33,12 @@
>> #include "texobj.h"
>> #include "fbobject.h"
>> #include "textureview.h"
>> +#include "glformats.h"
>> +
>> +enum mesa_block_class {
>> + BLOCK_CLASS_128_BITS,
>> + BLOCK_CLASS_64_BITS
>> +};
>>
>> static bool
>> prepare_target(struct gl_context *ctx, GLuint name, GLenum *target, int
>> level,
>> @@ -253,6 +259,124 @@ check_region_bounds(struct gl_context *ctx,
>> struct gl_texture_image *tex_image,
>> return true;
>> }
>>
>> +static bool
>> +compressed_format_compatible(struct gl_context *ctx,
>> + GLenum compressedFormat, GLenum otherFormat)
>> +{
>> + enum mesa_block_class compressedClass, otherClass;
>> +
>> + /* Two view-incompatible compressed formats are never compatible. */
>> + if (_mesa_is_compressed_format(ctx, otherFormat)) {
>> + return false;
>> + }
>> +
>> + /*
>> +* From ARB_copy_image spec:
>> +*Table 4.X.1 (Compatible internal formats for copying between
>> +* compressed and uncompressed internal formats)
>> +*
>> -
>> +*| Texel / | Uncompressed |
>> |
>> +*| Block | internal format | Compressed internal format
>> |
>> +*| size| |
>> |
>> +*
>> -
>> +*| 128-bit | RGBA32UI, | COMPRESSED_RGBA_S3TC_DXT3_EXT,
>> |
>> +*| | RGBA32I, |
>> COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT,|
>> +*| | RGBA32F | COMPRESSED_RGBA_S3TC_DXT5_EXT,
>> |
>> +*| | |
>> COMPRESSED_SRGB_ALPHA_S3TC_DXT5_EXT,|
>> +*| | | COMPRESSED_RG_RGTC2,
>> |
>> +*| | | COMPRESSED_SIGNED_RG_RGTC2,
>> |
>> +*| | | COMPRESSED_RGBA_BPTC_UNORM,
>> |
>> +*| | |
>> COMPRESSED_SRGB_ALPHA_BPTC_UNORM, |
>> +*| | |
>> COMPRESSED_RGB_BPTC_SIGNED_FLOAT, |
>> +*| | |
>> COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT |
>> +*
>> -
>> +*| 64-bit | RGBA16F, RG32F, | COMPRESSED_RGB_S3TC_DXT1_EXT,
>> |
>> +*| | RGBA16UI, RG32UI, | COMPRESSED_SRGB_S3TC_DXT1_EXT,
>> |
>> +*| | RGBA16I, RG32I, | COMPRESSED_RGBA_S3TC_DXT1_EXT,
>> |
>> +*| | RGBA16, |
>> COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT,|
>> +*| | RGBA16_SNORM | COMPRESSED_RED_RGTC1,
>> |
>> +*| | | COMPRESSED_SIGNED_RED_RGTC1
>> |
>> +*
>> -
>> +*/
>> +
>> + switch (compressedFormat) {
>> + case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
>> + case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT3_EXT:
>> + case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
Re: [Mesa-dev] [PATCH] util: convert slab macros to inline functions
On 03/12/2015 05:03 PM, Matt Turner wrote: On Thu, Mar 12, 2015 at 2:54 PM, Brian Paul wrote: --- src/gallium/auxiliary/util/u_slab.h | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_slab.h b/src/gallium/auxiliary/util/u_slab.h index 29d0252..8f8b29b 100644 --- a/src/gallium/auxiliary/util/u_slab.h +++ b/src/gallium/auxiliary/util/u_slab.h @@ -81,7 +81,16 @@ void util_slab_destroy(struct util_slab_mempool *pool); void util_slab_set_thread_safety(struct util_slab_mempool *pool, enum util_slab_threading threading); -#define util_slab_alloc(pool) (pool)->alloc(pool) -#define util_slab_free(pool, ptr) (pool)->free(pool, ptr) +static INLINE void * Maybe I'm making this up, but I was thinking there had been some work toward replacing INLINE with inline. If so, is this a candidate to use inline? In the gallium code it's still INLINE everywhere. If someone's looking for a relatively easy task, s/INLINE/inline/ in src/gallium/ could be a good project. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] util: convert slab macros to inline functions
On Thu, Mar 12, 2015 at 2:54 PM, Brian Paul wrote: > --- > src/gallium/auxiliary/util/u_slab.h | 13 +++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/auxiliary/util/u_slab.h > b/src/gallium/auxiliary/util/u_slab.h > index 29d0252..8f8b29b 100644 > --- a/src/gallium/auxiliary/util/u_slab.h > +++ b/src/gallium/auxiliary/util/u_slab.h > @@ -81,7 +81,16 @@ void util_slab_destroy(struct util_slab_mempool *pool); > void util_slab_set_thread_safety(struct util_slab_mempool *pool, > enum util_slab_threading threading); > > -#define util_slab_alloc(pool) (pool)->alloc(pool) > -#define util_slab_free(pool, ptr) (pool)->free(pool, ptr) > +static INLINE void * Maybe I'm making this up, but I was thinking there had been some work toward replacing INLINE with inline. If so, is this a candidate to use inline? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Matt Turner wrote: > I think you misread. rint() *does* provide the behavior we want > (round-to-nearest, half to even) when the rounding mode is the default > round-to-nearest. Thanks. I did at least verify that behaviorally as I just mentioned in a separate mail. > As Eric noted in his original patch, the man pages for > rint(3)/nearbyint(3) don't describe the half-to-even behavior but > round(3) does: Ah. Yes, I was using the man pages, so that was the source of my confusion. > Maybe I should send a patch to the Linux man-pages project too. That definitely wouldn't hurt. >> assert (fegetround() == FE_TONEAREST); > > I don't really want to do this. We rely on the default rounding mode > everywhere. I don't think asserting here is useful. What's useful is increasing the likelihood that when somebody does violate the assumption, they are alterted to the issue, rather than having Mesa quietly misbehave in hard-to-predict ways. It seems obvious to me that we should improve robustness when we've already identified exactly where (at least one piece) of fragility lies in Mesa. > Also it requires adding code for MSVC since it doesn't have > fegetround(). Not code, no. I'd be happy with just the #ifndef to hide the call from MSVC. I won't ask you find equivalent functionality for MSVC. -Carl pgpWezL81msfM.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Defer the throttle until we submit new commands
On 03/11/2015 05:36 AM, Chris Wilson wrote:
> Currently, we throttle before the user begins preparing commands for the
> next frame when we acquire the draw/read buffers. However, construction
> of the command buffer can itself take significant time relative to the
> frame time. If we move the throttle from the buffer acquire to the
> command submit phase we can allow the user to improve concurrency
> between the CPU and GPU (i.e. reduce the amount of time we waste inside
> the throttle).
Yes, more concurrency please. If Mesa is going to throttle, then throttling
should
happen definitely immediately before submitting the batch, not immediately
before
the user wants to use the cpu to build a new batch.
This patch solves my major complaint with the original patches you sent.
Reviewed-by: Chad Versace
But... please fix the style conventions in my comments below. Either way,
my r-b still stands.
> Signed-off-by: Chris Wilson
> Cc: Daniel Vetter
> Cc: Kenneth Graunke
> Cc: Ben Widawsky
> Cc: Kristian Høgsberg
> Cc: Chad Versace
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 34 --
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 41
> +++
> 2 files changed, 41 insertions(+), 34 deletions(-)
>
> @@ -226,6 +229,43 @@ brw_finish_batch(struct brw_context *brw)
> brw->cache.bo_used_by_gpu = true;
> }
>
> +static void throttle(struct brw_context *brw)
Convention in this file is:
static void
throttle(struct brw_context *brw)
> static int
> @@ -260,6 +300,7 @@ do_flush_locked(struct brw_context *brw)
>if (ret == 0) {
> if (unlikely(INTEL_DEBUG & DEBUG_AUB))
> brw_annotate_aub(brw);
> + throttle(brw);
>if (brw->hw_ctx == NULL || batch->ring != RENDER_RING) {
> ret = drm_intel_bo_mrb_exec(batch->bo, 4 * batch->used, NULL, 0, 0,
> flags);
Please put a newline above and below throttle(). Most i965 code is not so
compact.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: Create queryable strings in eglInitialize().
On 03/10/2015 09:15 PM, Matt Turner wrote: > On Tue, Mar 10, 2015 at 8:19 PM, Ian Romanick wrote: >> On 03/10/2015 05:20 PM, Matt Turner wrote: >>> Creating/recreating the strings in eglQueryString() is extra work and >>> isn't thread-safe, as exhibited by shader-db's run.c using libepoxy. >>> >>> Multiple threads in run.c call eglReleaseThread() around the same time. >>> libepoxy calls eglQueryString() to determine whether eglReleaseThread() >>> exists, and our EGL implementation passes a pointer to the version >>> string to libepoxy while simultaneously overwriting the string, leading >>> to a failure in libepoxy. >>> >>> Moreover, the EGL spec says (emphasis mine): >>> >>> "eglQueryString returns a pointer to a *static*, zero-terminated string" >>> >>> This patch moves some auxiliary functions from eglmisc.c to eglapi.c so >>> that they may be used to create the extension, API, and version strings >>> once during eglInitialize(). The auxiliary functions are renamed from >>> _eglUpdate* to _eglCreate*, and some checks made unnecessary by calling >>> the functions from eglInitialize() are removed. >>> >>> It was suggested to me to simply make the generation of the version >>> string behave like that of the extension and API strings -- to do a >>> check like >>> >>>if (exts[0]) >>> return; >>> >>> before creating the string, but that is not thread-safe either. >> >> It is safe because eglQueryString calls _eglQueryString (via >> drv->API.QueryString) inside a _eglLockDisplay region. > > Oh, you're right. I'll drop that bit from the commit message, pending > removal of API.QueryString. I like Matt's approach better (creating the strings during eglInitialize) because its correctness does not rely on the display lock. The big display lock is embarrassing, and we should avoid writing code that depends on it whenever possible. (Aside: The big display lock will probably cause trouble when implementing EGL_KHR_fence_sync, because that extension expects the display to be re-entrant with regard to syncs). >> The method you use here presumes that every implementation will use the >> _eglQueryString fallback for drv->API.QueryString. Right now it does, >> and I suspect that they always will. A little git archaeology leads me >> to believe that this has always been the case. >> >> Perhaps we should just remove API.QueryString altogether? Perhaps Brian >> knows of a reason not to... > > Getting rid of that could be nice. We could inline _eglQueryString, > avoid locking the display, and throw out eglmisc.c/eglmisc.h. Yes, let's get rid of API.QueryString. Mesa's egl/main directory contains a superfluous amount of virtual dispatch, and API.QueryString is one of the guilty parties. I like this patch is. It's Reviewed-by: Chad Versace ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mapi: Make private copies of name strings provided by client.
glXGetProcAddress("glFoo") ends up in stub_add_dynamic() to
create dynamic stubs for dynamic functions. stub_add_dynamic()
doesn't store the caller provided name string "Foo" in a mesa
private copy, but just stores a pointer to the "glFoo" string
passed to glXGetProcAddress - a pointer into arbitrary memory
outside mesa's control.
If the caller passes some dynamically allocated/changing
memory buffer to glXGetProcAddress(), or the caller gets unmapped
from memory, e.g., some dynamically loaded application
plugin which uses OpenGL, this ends badly - with a dangling
pointer.
strdup() the name string provided by the client to avoid
this problem.
Cc: "10.3 10.4 10.5"
Signed-off-by: Mario Kleiner
---
src/mapi/stub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mapi/stub.c b/src/mapi/stub.c
index dfadbe1..45ccc6a 100644
--- a/src/mapi/stub.c
+++ b/src/mapi/stub.c
@@ -110,7 +110,7 @@ stub_add_dynamic(const char *name)
if (!stub->addr)
return NULL;
- stub->name = (const void *) name;
+ stub->name = (const void *) strdup(name);
/* to be fixed later */
stub->slot = -1;
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12, 2015 at 2:59 PM, Carl Worth wrote:
> On Thu, Mar 12 2015, Matt Turner wrote:
>> +/* The C standard library has functions round()/rint()/nearbyint() that
>> round
>> + * their arguments according to the rounding mode set in the floating-point
>> + * control register. While there are trunc()/ceil()/floor() functions that
>> do
>> + * a specific operation without modifying the rounding mode, there is no
>> + * roundeven() in any version of C.
>> + *
>> + * Technical Specification 18661 (ISO/IEC TS 18661-1:2014) adds roundeven(),
>> + * but it's unfortunately not implemented by glibc.
>> + *
>> + * This implementation differs in that it does not raise the inexact
>> exception.
>> + */
>
> This documentation needs the actual description of the behavior of the
> function. Most of the above comment is commentary on the implementation
> itself.
>
> Perhaps something like the following?
>
> Round \x to the nearest even integer (returned in floating-point
> format).
>
> Note: This function is similar to roundeven() as specified by
> Technical Specification 18661 (ISO/IEC TS 18661-1:2014),
> differing only in that _mesa_roundeven() won't necessarily raise
> the inexact exception as roundeven() would.
Sounds good.
> Then, (within the function body itself?), it makes sense to have a
> comment on the implementation:
>
> When glibc eventually implements the standardized roundeven() we
> could use it directly. Until then, the available C standard
> library functions have the following limitations:
>
> round()/rint()/nearbyint(): Behavior varies depending on
> the rounding mode set in the floating-point control
> register.
>
> trunc()/ceil()/floor(): These specify rounding without
> relying on the rounding mode, but none give the rounding
> behavior desired for _mesa_roundeven().
I don't know why we would want to document that these functions don't
implement the behavior we want. That seems obvious enough...
>
I'm not sure what you mean. My intention in noting the existence of
trunc/ceil/floor is to note that there isn't an analogous function
with round-to-even behavior -- we have to use rint() with a particular
rounding mode.
>> +static inline float
>> +_mesa_roundevenf(float x)
>> +{
>> + float ret;
>> + /* Assume that the floating-point rounding mode has not been changed from
>> +* the default (Round to nearest).
>> +*/
>> + ret = rintf(x);
>> + return ret;
>> +}
>
> But after all that commentary about how rint() doesn't provide what's
> needed, here the implementation is just using it anyway?
I think you misread. rint() *does* provide the behavior we want
(round-to-nearest, half to even) when the rounding mode is the default
round-to-nearest.
As Eric noted in his original patch, the man pages for
rint(3)/nearbyint(3) don't describe the half-to-even behavior but
round(3) does:
> These functions round x to the nearest integer, but round halfway
> cases away from zero (regardless of the current rounding direction, see
> fenv(3)), instead of to the nearest even integer like rint(3).
Maybe I should send a patch to the Linux man-pages project too.
> The comment here, (and even any history of "other parts of mesa make the
> same assumption"), doesn't give the code adequate robustness. So at
> least an assert is needed here. Is this correct: ?
>
> assert (fegetround() == FE_TONEAREST);
I don't really want to do this. We rely on the default rounding mode
everywhere. I don't think asserting here is useful. Also it requires
adding code for MSVC since it doesn't have fegetround().
> But beyond that, I'm actually confused---how can the default rounding
> mode (rounding toward nearest number, right?) give us an adequate
> implementation for roundeven()?
Answered above.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Carl Worth wrote: > But beyond that, I'm actually confused---how can the default rounding > mode (rounding toward nearest number, right?) give us an adequate > implementation for roundeven()? Of course, I had asked for (and received) a patch with a test. And the test verifies that rint() with the default rounding is working. (I also verified that with an explicit call to fesetround(FE_TONEAREST) in the test, the test still passes, and similarly that a call to fesetround with any of the other values makes it fail.) So I'll clarify my confusion separately, but it doesn't necessarily point to any problem in the code. -Carl PS. And with that testing, here's my official: Reviewed-by: Carl Worth for the testing patch. pgpnpX_hGkdI4.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89563] Bug in configure verifying xcb version
https://bugs.freedesktop.org/show_bug.cgi?id=89563 MichaelJ changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG --- Comment #4 from MichaelJ --- Thanks. I will thus need to find the package outside of yum. yum is so easy... Since it is not a bug I will mark it solved. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Matt Turner wrote:
> +/* The C standard library has functions round()/rint()/nearbyint() that round
> + * their arguments according to the rounding mode set in the floating-point
> + * control register. While there are trunc()/ceil()/floor() functions that do
> + * a specific operation without modifying the rounding mode, there is no
> + * roundeven() in any version of C.
> + *
> + * Technical Specification 18661 (ISO/IEC TS 18661-1:2014) adds roundeven(),
> + * but it's unfortunately not implemented by glibc.
> + *
> + * This implementation differs in that it does not raise the inexact
> exception.
> + */
This documentation needs the actual description of the behavior of the
function. Most of the above comment is commentary on the implementation
itself.
Perhaps something like the following?
Round \x to the nearest even integer (returned in floating-point
format).
Note: This function is similar to roundeven() as specified by
Technical Specification 18661 (ISO/IEC TS 18661-1:2014),
differing only in that _mesa_roundeven() won't necessarily raise
the inexact exception as roundeven() would.
Then, (within the function body itself?), it makes sense to have a
comment on the implementation:
When glibc eventually implements the standardized roundeven() we
could use it directly. Until then, the available C standard
library functions have the following limitations:
round()/rint()/nearbyint(): Behavior varies depending on
the rounding mode set in the floating-point control
register.
trunc()/ceil()/floor(): These specify rounding without
relying on the rounding mode, but none give the rounding
behavior desired for _mesa_roundeven().
> +static inline float
> +_mesa_roundevenf(float x)
> +{
> + float ret;
> + /* Assume that the floating-point rounding mode has not been changed from
> +* the default (Round to nearest).
> +*/
> + ret = rintf(x);
> + return ret;
> +}
But after all that commentary about how rint() doesn't provide what's
needed, here the implementation is just using it anyway?
The comment here, (and even any history of "other parts of mesa make the
same assumption"), doesn't give the code adequate robustness. So at
least an assert is needed here. Is this correct: ?
assert (fegetround() == FE_TONEAREST);
But beyond that, I'm actually confused---how can the default rounding
mode (rounding toward nearest number, right?) give us an adequate
implementation for roundeven()?
[Thanks for sending the split patch. It made this question very
apparent, where I wasn't asking it about the combined series. Thanks
also for adding the test and the SSE optimization as a separate
commit. Those look good to me.]
-Carl
pgpKohQEREqPH.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] util: convert slab macros to inline functions
---
src/gallium/auxiliary/util/u_slab.h | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/gallium/auxiliary/util/u_slab.h
b/src/gallium/auxiliary/util/u_slab.h
index 29d0252..8f8b29b 100644
--- a/src/gallium/auxiliary/util/u_slab.h
+++ b/src/gallium/auxiliary/util/u_slab.h
@@ -81,7 +81,16 @@ void util_slab_destroy(struct util_slab_mempool *pool);
void util_slab_set_thread_safety(struct util_slab_mempool *pool,
enum util_slab_threading threading);
-#define util_slab_alloc(pool) (pool)->alloc(pool)
-#define util_slab_free(pool, ptr) (pool)->free(pool, ptr)
+static INLINE void *
+util_slab_alloc(struct util_slab_mempool *pool)
+{
+ return pool->alloc(pool);
+}
+
+static INLINE void
+util_slab_free(struct util_slab_mempool *pool, void *ptr)
+{
+ pool->free(pool, ptr);
+}
#endif
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89563] Bug in configure verifying xcb version
https://bugs.freedesktop.org/show_bug.cgi?id=89563 --- Comment #3 from Matt Turner --- This sounds to me like you have 1.9 with some patches applied. If that's the case, configure is rightly failing. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89563] Bug in configure verifying xcb version
https://bugs.freedesktop.org/show_bug.cgi?id=89563 --- Comment #2 from MichaelJ --- Response for this command is 1.9 Yum reports package 1.9-5.el7. this is latest available for my yum repositories. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89563] Bug in configure verifying xcb version
https://bugs.freedesktop.org/show_bug.cgi?id=89563 --- Comment #1 from Jan Vesely --- (In reply to MichaelJ from comment #0) > Incorrect error message "DRI3 requires xcb >= 1.9.3" when installed version > of libxcb is 1.9-5, later then 1.9.3. > "libxcb -v" returns 1.9-5.el7 but is parsed by configure to 1.9 thus > failing compare to required $XCB_REQUIRED set to 1.9.3. > libxcb version 1.9-5 is available in yum repositories. The code responsible > for this verification is around line 9 of configure file. I'm not fluent > in this script to attempt fix it myself. My work around was to set > XCB_REQUIRED to 1.9, line 5264, but in my opinion it is not acceptable > permanent fix. can you check the pkg-config reported version? like this: pkg-config --modversion xcb The latest released version (in 1.9 branch) is 1.9.3 if I remember correctly -5 (note "-" instead of ".") means 5th RH build of the version before "-". In you case it'd be 5th build of libxcb 1.9. the above command will tell. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] util: Optimize _mesa_roundeven with SSE 4.1.
On Thu, Mar 12, 2015 at 1:11 PM, Ilia Mirkin wrote:
> On Thu, Mar 12, 2015 at 3:11 PM, Matt Turner wrote:
>> The SSE 4.1 ROUND instructions let us implement roundeven directly.
>> Otherwise we assume that the rounding mode has not been modified (as we
>> do in the rest of Mesa) and use rint().
>>
>> glibc uses the ROUND instruction in rint() after a cpuid check. This
>> patch just lets us inline it directly when we're already building for
>> SSE 4.1.
>> ---
>> src/util/rounding.h | 16
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/src/util/rounding.h b/src/util/rounding.h
>> index d128524..e82fb59 100644
>> --- a/src/util/rounding.h
>> +++ b/src/util/rounding.h
>> @@ -23,6 +23,10 @@
>>
>> #include
>>
>> +#ifdef __SSE4_1__
>> +#include
>> +#endif
>> +
>> /* The C standard library has functions round()/rint()/nearbyint() that
>> round
>> * their arguments according to the rounding mode set in the floating-point
>> * control register. While there are trunc()/ceil()/floor() functions that
>> do
>> @@ -38,10 +42,16 @@ static inline float
>> _mesa_roundevenf(float x)
>> {
>> float ret;
>> +#ifdef __SSE4_1__
>> + __m128 m = _mm_load_ss(&x);
>> + m = _mm_round_ss(m, m, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC);
>> + _mm_store_ss(&ret, m);
>
> As we discussed, this will work differently than glibc depending on
> the rounding mode set. Either use the same rounding mode that glibc
> would use, or add an assert that the rounding mode is set the way you
> expect (and if it doesn't work on msvc, then... wtvr. That's what
> ifdef's are for.)
Sure. I'll change _MM_FROUND_TO_NEAREST_INT to _MM_FROUND_CUR_DIRECTION.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89563] Bug in configure verifying xcb version
https://bugs.freedesktop.org/show_bug.cgi?id=89563 Bug ID: 89563 Summary: Bug in configure verifying xcb version Product: Mesa Version: 10.5 Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Other Assignee: mesa-dev@lists.freedesktop.org Reporter: emjot...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org Incorrect error message "DRI3 requires xcb >= 1.9.3" when installed version of libxcb is 1.9-5, later then 1.9.3. "libxcb -v" returns 1.9-5.el7 but is parsed by configure to 1.9 thus failing compare to required $XCB_REQUIRED set to 1.9.3. libxcb version 1.9-5 is available in yum repositories. The code responsible for this verification is around line 9 of configure file. I'm not fluent in this script to attempt fix it myself. My work around was to set XCB_REQUIRED to 1.9, line 5264, but in my opinion it is not acceptable permanent fix. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] New stable-branch 10.5 candidate pushed
That's okay, just wanted to make sure that 10.5 was correct. On Thu, Mar 12, 2015 at 1:49 PM, Emil Velikov wrote: > On 12/03/15 16:55, Laura Ekstrand wrote: > > Emil, > > > > I spoke with Neil Roberts, and the two patches of mine you have listed > > as rejected here are required basis for his four queued commits (all of > > these pertain to meta_pbo_texSubImage). The only patch that was > > reverted was my "1D ARRAY" patch. > > > Hi Laura, > > It seems that the summary incorrectly lists your two patches are > rejected. Namely they are worthy 10.5 material and have landed prior to > the 10.5.0 release. I'm not entirely sure how I've managed to list them > here. > > Apologies for the confusion. > > Emil > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] New stable-branch 10.5 candidate pushed
On 12/03/15 16:55, Laura Ekstrand wrote: > Emil, > > I spoke with Neil Roberts, and the two patches of mine you have listed > as rejected here are required basis for his four queued commits (all of > these pertain to meta_pbo_texSubImage). The only patch that was > reverted was my "1D ARRAY" patch. > Hi Laura, It seems that the summary incorrectly lists your two patches are rejected. Namely they are worthy 10.5 material and have landed prior to the 10.5.0 release. I'm not entirely sure how I've managed to list them here. Apologies for the confusion. Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] util: Optimize _mesa_roundeven with SSE 4.1.
On Thu, Mar 12, 2015 at 3:11 PM, Matt Turner wrote:
> The SSE 4.1 ROUND instructions let us implement roundeven directly.
> Otherwise we assume that the rounding mode has not been modified (as we
> do in the rest of Mesa) and use rint().
>
> glibc uses the ROUND instruction in rint() after a cpuid check. This
> patch just lets us inline it directly when we're already building for
> SSE 4.1.
> ---
> src/util/rounding.h | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/src/util/rounding.h b/src/util/rounding.h
> index d128524..e82fb59 100644
> --- a/src/util/rounding.h
> +++ b/src/util/rounding.h
> @@ -23,6 +23,10 @@
>
> #include
>
> +#ifdef __SSE4_1__
> +#include
> +#endif
> +
> /* The C standard library has functions round()/rint()/nearbyint() that round
> * their arguments according to the rounding mode set in the floating-point
> * control register. While there are trunc()/ceil()/floor() functions that do
> @@ -38,10 +42,16 @@ static inline float
> _mesa_roundevenf(float x)
> {
> float ret;
> +#ifdef __SSE4_1__
> + __m128 m = _mm_load_ss(&x);
> + m = _mm_round_ss(m, m, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC);
> + _mm_store_ss(&ret, m);
As we discussed, this will work differently than glibc depending on
the rounding mode set. Either use the same rounding mode that glibc
would use, or add an assert that the rounding mode is set the way you
expect (and if it doesn't work on msvc, then... wtvr. That's what
ifdef's are for.)
> +#else
> /* Assume that the floating-point rounding mode has not been changed from
> * the default (Round to nearest).
> */
> ret = rintf(x);
> +#endif
> return ret;
> }
>
> @@ -49,9 +59,15 @@ static inline double
> _mesa_roundeven(double x)
> {
> double ret;
> +#ifdef __SSE4_1__
> + __m128d m = _mm_load_sd(&x);
> + m = _mm_round_sd(m, m, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC);
> + _mm_store_sd(&ret, m);
> +#else
> /* Assume that the floating-point rounding mode has not been changed from
> * the default (Round to nearest).
> */
> ret = rint(x);
> +#endif
> return ret;
> }
> --
> 2.0.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Revert "main: _mesa_cube_level_complete checks NumLayers."
It appears that our Piglit tests that hit glGenerateMipmap always create the cube map texture with Texture Storage, so Num Layers always gets set to 6. I will make a simple test this afternoon for non texture storage cube maps. On Mar 12, 2015 12:33 PM, "Laura Ekstrand" wrote: > This reverts commit 1ee000a0b6737d6c140d4f07b6044908b8ebfdc7. > Failures with the GLES3 conformance suite and Synmark2 OGLHdrBloom revealed > that this commit might be in error. A look at the offended test in GLES3 > conformance suite, NPOT gen mipmap, suggests that NumLayers may not > actually > always be 6 for a cube complete texture with target = GL_TEXTURE_CUBE_MAP. > > Extensive testing with Piglit prior to patch review and upstreaming did not > reveal this problem because, in the few Piglit tests that test for cube > completeness, NumLayers = 6. So it appears that perhaps existing test > coverage in Piglit is inadequate. > --- > src/mesa/main/texobj.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c > index a99b108..e018ab9 100644 > --- a/src/mesa/main/texobj.c > +++ b/src/mesa/main/texobj.c > @@ -879,10 +879,6 @@ _mesa_cube_level_complete(const struct > gl_texture_object *texObj, > if (texObj->Target != GL_TEXTURE_CUBE_MAP) >return GL_FALSE; > > - /* Make sure we have enough image planes for a cube map. */ > - if (texObj->NumLayers < 6) > - return GL_FALSE; > - > if ((level < 0) || (level >= MAX_TEXTURE_LEVELS)) >return GL_FALSE; > > -- > 2.1.0 > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Resend [PATCH] glx: Handle out-of-sequence swap completion events correctly.
Hi all, a respin of the bugfix for INTEL_swap_events + DRI3/Present. The code in the patch itself is identical to the one queued for Mesa 10.5.1, the one apparently nobody wants to review - I hate wraparound handling too... The only difference is a much longer commit message which explains why we need this for DRI3/Present, and why this new wraparound handling is supposed to work. If nobody wants to review this, or can come up with a better solution, i could also submit a patch that simply removes the broken wraparound handling of current Mesa, which would still be an improvement over what we have now, given that few OpenGL clients will submit over 2^32 swapbuffers in a single session for a single drawable, as that would require a runtime without interruption of over 13 months at 120 fps redraw rate. thanks, -mario ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glx: Handle out-of-sequence swap completion events correctly. (v2)
The code for emitting INTEL_swap_events swap completion events needs to translate from 32-Bit sbc on the wire to 64-Bit sbc for the events and handle wraparound accordingly. It assumed that events would be sent by the server in the order their corresponding swap requests were emitted from the client, iow. sbc count should be always increasing. This was correct for DRI2. This is not always the case under the DRI3/Present backend, where the Present extension can execute presents and send out completion events in a different order than the submission order of the present requests, due to client code specifying targetMSC target vblank counts which are not strictly monotonically increasing. This confused the wraparound handling. This patch fixes the problem by handling 32-Bit wraparound in both directions. As long as successive swap completion events real 64-Bit sbc's don't differ by more than 2^30, this should be able to do the right thing. How this is supposed to work: awire->sbc contains the low 32-Bits of the true 64-Bit sbc of the current swap event, transmitted over the wire. glxDraw->lastEventSbc contains the low 32-Bits of the 64-Bit sbc of the most recently processed swap event. glxDraw->eventSbcWrap is a 64-Bit offset which tracks the upper 32-Bits of the current sbc. The final 64-Bit output sbc aevent->sbc is computed from the sum of awire->sbc and glxDraw->eventSbcWrap. Under DRI3/Present, swap completion events can be received slightly out of order due to non-monotic targetMsc specified by client code, e.g., present request submission: Submission sbc: 1 2 3 targetMsc:10 11 9 Reception of completion events: Completion sbc: 3 1 2 The completion sequence 3, 1, 2 would confuse the old wraparound handling made for DRI2 as 1 < 3 --> Assumes a 32-Bit wraparound has happened when it hasn't. The client can queue multiple present requests, in the case of Mesa up to n requests for n-buffered rendering, e.g., n = 2-4 in the current Mesa GLX DRI3/Present implementation. In the case of direct Pixmap presents via xcb_present_pixmap() the number n is limited by the amount of memory available. We reasonably assume that the number of outstanding requests n is much less than 2 billion due to memory contraints and common sense. Therefore while the order of received sbc's can be a bit scrambled, successive 64-Bit sbc's won't deviate by much, a given sbc may be a few counts lower or higher than the previous received sbc. Therefore any large difference between the incoming awire->sbc and the last recorded glxDraw->lastEventSbc will be due to 32-Bit wraparound and we need to adapt glxDraw->eventSbcWrap accordingly to adjust the upper 32-Bits of the sbc. Two cases, correponding to the two if-statements in the patch: a) Previous sbc event was below the last 2^32 boundary, in the previous glxDraw->eventSbcWrap epoch, the new sbc event is in the next 2^32 epoch, therefore the low 32-Bit awire->sbc wrapped around to zero, or close to zero --> awire->sbc is apparently much lower than the glxDraw->lastEventSbc recorded for the previous epoch --> We need to increment glxDraw->eventSbcWrap by 2^32 to adjust the current epoch to be one higher than the previous one. --> Case a) also handles the old DRI2 behaviour. b) Previous sbc event was above closest 2^32 boundary, but now a late event from the previous 2^32 epoch arrives, with a true sbc that belongs to the previous 2^32 segment, so the awire->sbc of this late event has a high count close to 2^32, whereas glxDraw->lastEventSbc is closer to zero --> awire->sbc is much greater than glXDraw->lastEventSbc. --> We need to decrement glxDraw->eventSbcWrap by 2^32 to adjust the current epoch back to the previous lower epoch of this late completion event. We assume such a wraparound to a higher (a) epoch or lower (b) epoch has happened if awire->sbc and glxDraw->lastEventSbc differ by more than 2^30 counts, as such a difference can only happen on wraparound, or if somehow 2^30 present requests would be pending for a given drawable inside the server, which is rather unlikely. v2: Explain the reason for this patch and the new wraparound handling much more extensive in commit message, no code change wrt. initial version. Cc: "10.3 10.4 10.5" Signed-off-by: Mario Kleiner --- src/glx/glxext.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/glx/glxext.c b/src/glx/glxext.c index 68c359e..fdc24d4 100644 --- a/src/glx/glxext.c +++ b/src/glx/glxext.c @@ -143,8 +143,13 @@ __glXWireToEvent(Display *dpy, XEvent *event, xEvent *wire) aevent->ust = ((CARD64)awire->ust_hi << 32) | awire->ust_lo; aevent->msc = ((CARD64)awire->msc_hi << 32) | awire->msc_lo; - if (awire->sbc < glxDraw->lastEventSbc) -glxDraw->eventSbcWrap += 0x1; + /* Handle 32-Bit wire sbc wraparound in both directions to cope with out + * of sequence 64-Bit sbc's + */ + if ((int64_t) awire->s
[Mesa-dev] [PATCH] Revert "main: _mesa_cube_level_complete checks NumLayers."
This reverts commit 1ee000a0b6737d6c140d4f07b6044908b8ebfdc7. Failures with the GLES3 conformance suite and Synmark2 OGLHdrBloom revealed that this commit might be in error. A look at the offended test in GLES3 conformance suite, NPOT gen mipmap, suggests that NumLayers may not actually always be 6 for a cube complete texture with target = GL_TEXTURE_CUBE_MAP. Extensive testing with Piglit prior to patch review and upstreaming did not reveal this problem because, in the few Piglit tests that test for cube completeness, NumLayers = 6. So it appears that perhaps existing test coverage in Piglit is inadequate. --- src/mesa/main/texobj.c | 4 1 file changed, 4 deletions(-) diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c index a99b108..e018ab9 100644 --- a/src/mesa/main/texobj.c +++ b/src/mesa/main/texobj.c @@ -879,10 +879,6 @@ _mesa_cube_level_complete(const struct gl_texture_object *texObj, if (texObj->Target != GL_TEXTURE_CUBE_MAP) return GL_FALSE; - /* Make sure we have enough image planes for a cube map. */ - if (texObj->NumLayers < 6) - return GL_FALSE; - if ((level < 0) || (level >= MAX_TEXTURE_LEVELS)) return GL_FALSE; -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/5] i965/fs: Apply gl_FrontFacing ? -1 : 1 optimization only for floats
On 03/11/2015 04:10 PM, Matt Turner wrote: > On Wed, Mar 11, 2015 at 1:44 PM, Ian Romanick wrote: >> From: Ian Romanick >> >> At the very least, unreal4/sun-temple/102.shader_test uses this pattern >> for a signed integer result. However, that shader did not hit the >> optimization in the first place because it uses !gl_FronFacing. I >> changed the shader to use remove the logical-not and reverse the other >> operands. I verified that incorrect code is generated before this >> change and correct code is generated after. > > At one point I did have a patch that handled !gl_FrontFacing ? ... but > I think it got lost. I have a patch (somewhere) that converts (!b ? x : y) to (b ? y : x). I think that would make explicit handling of !gl_FrontFacing unnecessary. I also have some plan to add support for values other than +/-1.0. I think other optimizations in my queue make other values occur. > 4-5 are > > Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] util: Optimize _mesa_roundeven with SSE 4.1.
The SSE 4.1 ROUND instructions let us implement roundeven directly.
Otherwise we assume that the rounding mode has not been modified (as we
do in the rest of Mesa) and use rint().
glibc uses the ROUND instruction in rint() after a cpuid check. This
patch just lets us inline it directly when we're already building for
SSE 4.1.
---
src/util/rounding.h | 16
1 file changed, 16 insertions(+)
diff --git a/src/util/rounding.h b/src/util/rounding.h
index d128524..e82fb59 100644
--- a/src/util/rounding.h
+++ b/src/util/rounding.h
@@ -23,6 +23,10 @@
#include
+#ifdef __SSE4_1__
+#include
+#endif
+
/* The C standard library has functions round()/rint()/nearbyint() that round
* their arguments according to the rounding mode set in the floating-point
* control register. While there are trunc()/ceil()/floor() functions that do
@@ -38,10 +42,16 @@ static inline float
_mesa_roundevenf(float x)
{
float ret;
+#ifdef __SSE4_1__
+ __m128 m = _mm_load_ss(&x);
+ m = _mm_round_ss(m, m, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC);
+ _mm_store_ss(&ret, m);
+#else
/* Assume that the floating-point rounding mode has not been changed from
* the default (Round to nearest).
*/
ret = rintf(x);
+#endif
return ret;
}
@@ -49,9 +59,15 @@ static inline double
_mesa_roundeven(double x)
{
double ret;
+#ifdef __SSE4_1__
+ __m128d m = _mm_load_sd(&x);
+ m = _mm_round_sd(m, m, _MM_FROUND_TO_NEAREST_INT | _MM_FROUND_NO_EXC);
+ _mm_store_sd(&ret, m);
+#else
/* Assume that the floating-point rounding mode has not been changed from
* the default (Round to nearest).
*/
ret = rint(x);
+#endif
return ret;
}
--
2.0.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Replace _mesa_round_to_even() with roundeven().
On Wed, Mar 11, 2015 at 3:34 PM, Carl Worth wrote: > So I think I'd like to see at least three or four commits here: > > 1. Change _mesa_round_to_even to return a float I don't think this is really meaningful as a separate change. The current _mesa_round_to_even function uses IROUND() which rounds to integer. I could do the conversion back to float in the function (that's occurring implicitly in ir_expression::constant_expression_value() already). We'd just be moving where the (bad) conversion is happening. Basically, the old thing is broken in multiple ways. Attempting to fix the individual bugs separately is more work than its worth when the new implementation is just rintf(x). > 2. Fix rounding bug in _mesa_round_to_even > 3. Add unit test for recent rounding-bug fix > 4. Optimize _mesa_round_to_even with SSE 4.1 ROUND if available Sure, these seem fine. I kind of agree that giving the new function the same name as something from a spec but giving it slightly different behavior might be bad. Keeping the same name also seems bad though -- changing a function that takes a float and returns an int to take and return a double (assuming the float and double implementations are named _mesa_round_to_evenf and _mesa_round_to_even). Ken suggested _mesa_roundevenf/_mesa_roundeven. I like this. It says it's the roundeven function, but doesn't claim that its completely identical. > With that: Reviewed-by: Carl Worth I can't really apply a review for a bunch of commits you haven't seen yet. :) I'll send the three updated/split patches. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] util: Add a roundeven test.
---
src/util/Makefile.am | 4 +-
src/util/roundeven_test.c | 140 ++
2 files changed, 143 insertions(+), 1 deletion(-)
create mode 100644 src/util/roundeven_test.c
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index ec49dc6..2e7542e 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -50,7 +50,9 @@ endif
libmesautil_la_LIBADD = $(SHA1_LIBS)
-check_PROGRAMS = u_atomic_test
+roundeven_test_LDADD = -lm
+
+check_PROGRAMS = u_atomic_test roundeven_test
TESTS = $(check_PROGRAMS)
BUILT_SOURCES = $(MESA_UTIL_GENERATED_FILES)
diff --git a/src/util/roundeven_test.c b/src/util/roundeven_test.c
new file mode 100644
index 000..7526db1
--- /dev/null
+++ b/src/util/roundeven_test.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "macros.h"
+#include "rounding.h"
+
+int main(int argc, char *argv[])
+{
+ const struct {
+ float input, expected;
+ } float_data[] = {
+ { 0.0, 0.0 },
+ { nextafterf(0.5, 0.0), 0.0 },
+ { 0.5, 0.0 },
+ { nextafterf(0.5, 1.0), 1.0 },
+ { 1.0, 1.0 },
+ { nextafterf(1.5, 1.0), 1.0 },
+ { 1.5, 2.0 },
+ { nextafterf(1.5, 2.0), 2.0 },
+ { 2.0, 2.0 },
+ { nextafterf(2.5, 2.0), 2.0 },
+ { 2.5, 2.0 },
+ { nextafterf(2.5, 3.0), 3.0 },
+ };
+
+ const struct {
+ double input, expected;
+ } double_data[] = {
+ { 0.0, 0.0 },
+ { nextafter(0.5, 0.0), 0.0 },
+ { 0.5, 0.0 },
+ { nextafter(0.5, 1.0), 1.0 },
+ { 1.0, 1.0 },
+ { nextafter(1.5, 1.0), 1.0 },
+ { 1.5, 2.0 },
+ { nextafter(1.5, 2.0), 2.0 },
+ { 2.0, 2.0 },
+ { nextafter(2.5, 2.0), 2.0 },
+ { 2.5, 2.0 },
+ { nextafter(2.5, 3.0), 3.0 },
+ };
+
+ bool failed = false;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(float_data); i++) {
+ float output = _mesa_roundevenf(float_data[i].input);
+ if (memcmp(&float_data[i].expected, &output, sizeof(float))) {
+ fprintf(stderr, "%d float: expected %f (%a) from "
+ "_mesa_roundevenf(%f (%a)) but got %f (%a)\n",
+ i,
+ float_data[i].expected,
+ float_data[i].expected,
+ float_data[i].input,
+ float_data[i].input,
+ output,
+ output);
+ failed = true;
+ }
+ }
+
+ /* Test negated values */
+ for (i = 0; i < ARRAY_SIZE(float_data); i++) {
+ float output = _mesa_roundevenf(-float_data[i].input);
+ float negated_expected = -float_data[i].expected;
+ if (memcmp(&negated_expected, &output, sizeof(float))) {
+ fprintf(stderr, "%d float: expected %f (%a) from "
+ "_mesa_roundevenf(%f (%a)) but got %f (%a)\n",
+ i,
+ negated_expected,
+ negated_expected,
+ -float_data[i].input,
+ -float_data[i].input,
+ output,
+ output);
+ failed = true;
+ }
+ }
+
+ for (i = 0; i < ARRAY_SIZE(double_data); i++) {
+ double output = _mesa_roundeven(double_data[i].input);
+ if (memcmp(&double_data[i].expected, &output, sizeof(double))) {
+ fprintf(stderr, "%d double: expected %f (%a) from "
+ "_mesa_roundeven(%f (%a)) but got %f (%a)\n",
+ i,
+ double_data[i].expected,
+ double_data[i].expected,
+ double_data[i].input,
+ double_data[i].input,
+ output,
+
[Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
Eric's initial patch adding constant expression evaluation for
ir_unop_round_even used nearbyint. The open-coded _mesa_round_to_even
implementation came about without much explanation after a reviewer
asked whether nearbyint depended on the application not modifying the
rounding mode. Of course (as Eric commented) we rely on the application
not changing the rounding mode from its default (round-to-nearest) in
many other places, including the IROUND function used by
_mesa_round_to_even!
Worse, IROUND() is implemented using the trunc(x + 0.5) trick which
fails for x = nextafterf(0.5, 0.0).
Still worse, _mesa_round_to_even unexpectedly returns an int. I suspect
that could cause problems when rounding large integral values not
representable as an int in ir_constant_expression.cpp's
ir_unop_round_even
evaluation. Its use of _mesa_round_to_even is clearly broken for doubles
(as noted during review).
The constant expression evaluation code for the packing built-in
functions also mistakenly assumed that _mesa_round_to_even returned a
float, as can be seen by the cast through a signed integer type to an
unsigned (since negative float -> unsigned conversions are undefined).
rint() and nearbyint() implement the round-half-to-even behavior we want
when the rounding mode is set to the default round-to-nearest. The only
difference between them is that nearbyint() raises the inexact
exception.
This patch implements _mesa_roundeven{f,}, a function similar to the
roundeven function added by a yet unimplemented technical specification
(ISO/IEC TS 18661-1:2014), with a small difference in behavior -- we
don't bother raising the inexact exception, which I don't think we care
about anyway.
At least recent Intel CPUs can quickly change a subset of the bits in
the x87 floating-point control register, but the exception mask bits are
not included. rint() does not need to change these bits, but nearbyint()
does (twice: save old, set new, and restore old) in order to raise the
inexact exception, which would incur some penalty.
---
src/glsl/ir_constant_expression.cpp | 18 +-
src/glsl/nir/nir_constant_expressions.py | 15 +
src/glsl/nir/nir_opcodes.py | 2 +-
src/mesa/main/imports.c | 25 ++
src/mesa/main/imports.h | 3 --
src/util/Makefile.sources| 1 +
src/util/rounding.h | 57
7 files changed, 81 insertions(+), 40 deletions(-)
create mode 100644 src/util/rounding.h
diff --git a/src/glsl/ir_constant_expression.cpp
b/src/glsl/ir_constant_expression.cpp
index 388c4c2..ecebc3c 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -35,6 +35,7 @@
#include
#include "main/core.h" /* for MAX2, MIN2, CLAMP */
+#include "util/rounding.h" /* for _mesa_roundeven */
#include "ir.h"
#include "glsl_types.h"
#include "program/hash_table.h"
@@ -245,8 +246,8 @@ pack_snorm_1x8(float x)
* We must first cast the float to an int, because casting a negative
* float to a uint is undefined.
*/
- return (uint8_t) (int8_t)
- _mesa_round_to_even(CLAMP(x, -1.0f, +1.0f) * 127.0f);
+ return (uint8_t) (int)
+ _mesa_roundevenf(CLAMP(x, -1.0f, +1.0f) * 127.0f);
}
/**
@@ -267,8 +268,8 @@ pack_snorm_1x16(float x)
* We must first cast the float to an int, because casting a negative
* float to a uint is undefined.
*/
- return (uint16_t) (int16_t)
- _mesa_round_to_even(CLAMP(x, -1.0f, +1.0f) * 32767.0f);
+ return (uint16_t) (int)
+ _mesa_roundevenf(CLAMP(x, -1.0f, +1.0f) * 32767.0f);
}
/**
@@ -322,7 +323,7 @@ pack_unorm_1x8(float x)
*
* packUnorm4x8: round(clamp(c, 0, +1) * 255.0)
*/
- return (uint8_t) _mesa_round_to_even(CLAMP(x, 0.0f, 1.0f) * 255.0f);
+ return (uint8_t) (int) _mesa_roundevenf(CLAMP(x, 0.0f, 1.0f) * 255.0f);
}
/**
@@ -340,7 +341,8 @@ pack_unorm_1x16(float x)
*
* packUnorm2x16: round(clamp(c, 0, +1) * 65535.0)
*/
- return (uint16_t) _mesa_round_to_even(CLAMP(x, 0.0f, 1.0f) * 65535.0f);
+ return (uint16_t) (int)
+ _mesa_roundevenf(CLAMP(x, 0.0f, 1.0f) * 65535.0f);
}
/**
@@ -733,9 +735,9 @@ ir_expression::constant_expression_value(struct hash_table
*variable_context)
case ir_unop_round_even:
for (unsigned c = 0; c < op[0]->type->components(); c++) {
if (op[0]->type->base_type == GLSL_TYPE_DOUBLE)
-data.d[c] = _mesa_round_to_even(op[0]->value.d[c]);
+data.d[c] = _mesa_roundeven(op[0]->value.d[c]);
else
-data.f[c] = _mesa_round_to_even(op[0]->value.f[c]);
+data.f[c] = _mesa_roundevenf(op[0]->value.f[c]);
}
break;
diff --git a/src/glsl/nir/nir_constant_expressions.py
b/src/glsl/nir/nir_constant_expressions.py
index 22bc4f0..bf82fe5 100644
--- a/src/glsl/nir/nir_constant_expressions.py
+++ b/src/gls
Re: [Mesa-dev] [PATCH] i965: Throttle rendering to an fbo
On Wed, Mar 11, 2015 at 09:10:01AM -0700, Ian Romanick wrote: > On 03/06/2015 06:30 AM, Daniel Vetter wrote: > > On Thu, Mar 05, 2015 at 02:38:44PM -0800, Ian Romanick wrote: > >> On 03/04/2015 10:28 AM, Chad Versace wrote: > >>> That text does not appear in the GL spec. When I read the manpage > >>> alongside > >>> the GL spec, to get a more complete context, I think the manpage contains > >>> that phrase simply to contrast with glFinish. In my reading, it does not > >>> imply that > >>> glFlush may wait for *some* previously issued GL commands to complete. > >> > >> glFlush was invented to support indirect rendering (especially to the > >> front buffer): it flushes the buffer in libGL to the xserver. If > >> you're making any other assumptions about what it does or does not do... > >> continue at your own peril. > > > > Well plan B is the kernel throttles for you. But that's going to cause a > > fuzz since at least for benchmarking the kernel kinda lacks the necessary > > information to make informed calls about when to throttle and how. > > Based on my (possibly flawed) understanding of the kernel commit message > the Chris previously cited, we only need to throttle when not throttling > would cause a problem. Is that correct? Based on the rest of my > understanding, Mesa doesn't have the right information either. > > > So is there no gl entry point in mesa we can abuse and make this happen? > > Citing the spec doesn't make the real world issue go away. > > I think we're at a place where real worlds collide. > > There are a lot of poorly written apps out there. Much of it is the > "beat on it until it works" syndrome. This results in a lot of glFlush > abuse. This is part of the reason that drivers for deferred rendering > GPUs generally just ignore glFlush. Unnecessary flushes REALLY hurt > performance on tilers, but they don't matter so much on immediate mode > renderers. > > Applications do FBO rendering for a lot of things that don't end up on > the screen. For example, some applications will load images from disk > and use FBO rendering to generate the mipmaps. Applications also do > varying amounts of FBO rendering for pre-passes, shadow map generation, > etc. A few applications also do FBO rendering as a sort of poor man's > compute shader. > > Now we have the collision. You have an game that is going to do a few > thousand draws to FBOs to generate mipmaps at start-up with a bunch of > (probably unnecessary) glFlush calls sprinkled around. Now somebody > gets a bug report that the game takes too long to load because it's > getting throttled. > > I think throttling submission in Mesa is the right place, but I think we > need additional information to know when it is necessary. Ok makes sense then that anything in glflush wouldn't be a good idea. The current iteration of stalling before we submit more work at least has the benefit of some relation with the workloads generated. I guess in the end we do indeed need more clueful throttling in the kernel (to prevent bad things from happening), with some additional throttlin on frame boundaries in mesa to etch out the last bit of interactive responsiveness. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
On 03/11/2015 09:40 AM, Ian Romanick wrote: On 03/11/2015 09:31 AM, Tobias Klausmann wrote: The problem in not forcing this to link statically is, that if a distribution decides to not use this static option, the problem persists on that distribution. On top every lib pulled in by steam from the system would need to be link statically, like mesa. Instead of fixing every lib steam pulls in (how many are there?), fix the steam runtime to Yeah, static linking is a terrible, partial solution. a) not ship libstdc++.so and libgcc_s.so and declare older version of these libs as not supported (thats what people do when they face the incompatibility problem with steams versions of these libs: just delete them from the runtime and everything is fine) Let's be 100% clear. This is NOT an option for Steam. They ship thousands of closed-source applications. These applications were built and tested against specific versions of specific libraries. Removing support for old run-times is equivalent to removing support for those applications. They can't tell their customers, "You upgraded your distro, so that game you paid money for no longer works. Tough break, kid." Quite true; developers build and test their games against this known set of libraries through the Steam Runtime toolchain. Expecting them to manage all of their dependencies (through bundling or static linking) is unreasonable; all other platforms they're used to dealing with solve this problem one way or another. Admittedly the current implementation of the Steam runtime is only a first step towards providing a solution, and some things could be done better, but this issue will stand regardless. The C++ runtime is a very complex beast and swapping it under an application that makes heavy use of it is prone to failure. We'll keep running games against the C++ runtime of the SDK version they were built against, forever. The LLVM-enabled variants of Mesa are the only implementations where this is an issue; this is actually causing affected users in the wild to switch to the Catalyst driver, since it (along with all others) goes out of its way to avoid this specific problem for this exact reason. Their only other option is to slam the whole Steam runtime off, which breaks a lot of games outright because they depend on some library that recent distros don't ship anymore, or moved to the new major version. Distributions _want_ this fixed, not the other way around. Thanks, - Pierre-Loup Forcing an application to use a newer run-time may work over the short term, but do you think that will still work in 5 years? It's possible, but it seems unlikely. And "seems to work" is not the same as "works all the way through the whole game every time." ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] nesa-10.4.4: gallivm/lp_bld_misc.cpp:503:38: error: no viable conversion from 'ShaderMemoryManager *' to 'std::unique_ptr'
On 12/03/15 18:07, Sedat Dilek wrote: On Fri, Mar 6, 2015 at 8:06 PM, Emil Velikov wrote: On 4 March 2015 at 18:07, Roland Scheidegger wrote: Am 04.03.2015 um 12:38 schrieb Jose Fonseca: On 04/03/15 02:00, Emil Velikov wrote: On 27 February 2015 at 23:28, Sedat Dilek wrote: On Mon, Feb 9, 2015 at 6:30 PM, Emil Velikov wrote: On 07/02/15 21:44, Sedat Dilek wrote: Hi, I was building mesa v10.4.4 with my llvm-toolchain v3.6.0rc2. My build breaks like this... ... Please cherry-pick... commit ef7e0b39a24966526b102643523feac765771842 "gallivm: Update for RTDyldMemoryManager becoming an unique_ptr." ..for mesa 10.4 Git branch. Hi Sedat, Picking a fix in a stable branch against a non-final release sounds like a no-go in our books. As the official llvm 3.6 rolls out we'll pick this fix for the stable branches - until then I would recommend (a) applying it locally or (b) using mesa from the 10.5 or master branch. Just FYI... [LLVMdev] LLVM 3.6 Release (see [1]). Please pick this patch for-10.4, thanks. As promised, mesa 10.4.6 will feature this. But is cross-porting this patch enough? As I said when this first issue was raised fixing the build with LLVM 3.6 is just half of the problem. It must also _run_ correctly. And building correctly doesn't necessarily means it will run correctly. That is, unless somebody actually ensures that all LLVM 3.6 related fixes have been crossported and that things run correctly, it is misleading to enable the build of Mesa 10.4.6 with LLVM 3.6. I don't know about radeon drivers, but at least from llvmpipe POV I simply don't have the time to do this (go through every LLVM 3.6 related patch, ensure they are all in 10.4.6, and test). I quickly went through the diffs between 10.4 branch, and found one such commit is missing: https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D74f505fa73eda0c9b5b1984bebb44cedac8e8794&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=mwtTmgdaduzVfVZMIvrzUZfNVLzbS9ds0ks0JUM84a8&s=Sil2uIufz_rVBrlIPFDCUMC6Wcsupo40k41-Sz85i9k&e= https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D85467&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=mwtTmgdaduzVfVZMIvrzUZfNVLzbS9ds0ks0JUM84a8&s=abTgehf51Ko6ywCLleOeHsxY6igdNHXG4W8PHws8MQU&e= But there might be more, and I don't know if crossporting this is safe or not. Therefore my stance for is that building Mesa stable releases with LLVM releases after the Mesa release was branched is still unsupported. If people want to do so, they will do at their own peril. And any incoming bugs will be "unsupported, use Mesa. If having a Mesa release capable of building LLVM 3.6 is so important, I think it might be easier/safer to just make a new release from a recent enough commit, than trying to backport it. This is quite right, the above commit is a must if you want to build with llvm 3.6. I am quite sure crossport should be safe (it missed the branch point of 10.4 just narrowly), and I don't think there's any other patches missing, but no guarantees... I think it is sort of unfortunate that the latest mesa release wouldn't run with the latest llvm release, but the fact remains that without testing this sounds all a bit risky... Thanks for the input gents. So the input so far we've got is that no-one is testing llvm 3.6 with mesa 10.4. I love to give it a spin, yet Archlinux doesn't have llvm 3.6 . There is also the double-free bug mentioned in https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D89387&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=mwtTmgdaduzVfVZMIvrzUZfNVLzbS9ds0ks0JUM84a8&s=ce5nAs88nlMkISXKFeoBDzTSKeR3Q9I6sV7AP9COtwQ&e= All that said, Sedat I will revert the commit and release 10.4.6 without it. On the positive side, mesa 10.5.0 is coming out later on today, which should work like a charm with llvm 3.6. As said I switched to mesa v10.5.0. Just FYI... Mesa Bug #89387 was fixed by [1]... commit 70dc8a9930f561d7ce6db7e58b5bc9b4d940e37b "gallivm: Prevent double delete on LLVM 3.6" - Sedat - [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_patch_-3Fid-3D70dc8a9930f561d7ce6db7e58b5bc9b4d940e37b&d=AwIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=mwtTmgdaduzVfVZMIvrzUZfNVLzbS9ds0ks0JUM84a8&s=G9Y99Ju79y8r7f-SEKkY3Fl0dpHU2jXKR3iDRmr7Rvs&e= On 12/03/15 18:07, Sedat Dilek wrote: On Fri, Mar 6, 2015 at 8:06 PM, Emil Velikov wrote: On 4 March 2015 at 18:07, Roland Scheidegger wrote: Am 04.03.2015 um 12:38 schrieb Jose Fonseca: On 04/03/15 02:00, Emil Velikov wrote: On 27 February 2015 at 23:28, Sedat Dilek wrote: On Mon, Feb 9, 2015 at 6:30 PM, Emil Velikov wrote: On 07/02/
Re: [Mesa-dev] [PATCH 11/13] i965: Add untyped surface write opcode.
On Fri, Feb 27, 2015 at 05:34:54PM +0200, Francisco Jerez wrote:
> ---
> src/mesa/drivers/dri/i965/brw_defines.h| 1 +
> src/mesa/drivers/dri/i965/brw_eu.h | 7 +++
> src/mesa/drivers/dri/i965/brw_eu_emit.c| 51
> ++
> src/mesa/drivers/dri/i965/brw_fs.cpp | 4 ++
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 6 +++
> .../drivers/dri/i965/brw_schedule_instructions.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_shader.cpp | 3 ++
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 6 +++
> 9 files changed, 81 insertions(+)
Just a few formatting nits:
Reviewed-by: Topi Pohjolainen
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 7660feb..e56f49c 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -904,6 +904,7 @@ enum opcode {
>
> SHADER_OPCODE_UNTYPED_ATOMIC,
> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> + SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
>
> SHADER_OPCODE_GEN4_SCRATCH_READ,
> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 9cc9123..cad956b 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -414,6 +414,13 @@ brw_untyped_surface_read(struct brw_compile *p,
> unsigned num_channels);
>
> void
> +brw_untyped_surface_write(struct brw_compile *p,
> + struct brw_reg payload,
> + struct brw_reg surface,
> + unsigned msg_length,
> + unsigned num_channels);
> +
> +void
> brw_pixel_interpolator_query(struct brw_compile *p,
> struct brw_reg dest,
> struct brw_reg mrf,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 34695bf..f5b8fa9 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2893,6 +2893,57 @@ brw_untyped_surface_read(struct brw_compile *p,
>p, insn, num_channels);
> }
>
> +static void
> +brw_set_dp_untyped_surface_write_message(struct brw_compile *p,
> + struct brw_inst *insn,
> + unsigned num_channels)
> +{
> + const struct brw_context *brw = p->brw;
> + /* Set mask of 32-bit channels to drop. */
> + unsigned msg_control = (0xf & (0xf << num_channels));
Could drop the extra () here.
> +
> + if (brw_inst_access_mode(brw, p->current) == BRW_ALIGN_1) {
> + if (p->compressed)
> + msg_control |= 1 << 4; /* SIMD16 mode */
> + else
> + msg_control |= 2 << 4; /* SIMD8 mode */
> + } else {
> + if (brw->gen >= 8 || brw->is_haswell)
> + msg_control |= 0 << 4; /* SIMD4x2 mode */
> + else
> + msg_control |= 2 << 4; /* SIMD8 mode */
> + }
> +
> + brw_inst_set_dp_msg_type(brw, insn,
> +(brw->gen >= 8 || brw->is_haswell ?
> + HSW_DATAPORT_DC_PORT1_UNTYPED_SURFACE_WRITE :
> + GEN7_DATAPORT_DC_UNTYPED_SURFACE_WRITE));
Same here.
> + brw_inst_set_dp_msg_control(brw, insn, msg_control);
> +}
> +
> +void
> +brw_untyped_surface_write(struct brw_compile *p,
> + struct brw_reg payload,
> + struct brw_reg surface,
> + unsigned msg_length,
> + unsigned num_channels)
> +{
> + const struct brw_context *brw = p->brw;
> + const unsigned sfid = (brw->gen >= 8 || p->brw->is_haswell ?
> + HSW_SFID_DATAPORT_DATA_CACHE_1 :
> + GEN7_SFID_DATAPORT_DATA_CACHE);
> + const bool align1 = (brw_inst_access_mode(brw, p->current) ==
> BRW_ALIGN_1);
And here.
> + /* Mask out unused components -- See comment in brw_untyped_atomic(). */
> + const unsigned mask = (brw->gen == 7 && !brw->is_haswell && !align1 ?
> + WRITEMASK_X : WRITEMASK_XYZW);
And here.
> + struct brw_inst *insn = brw_send_indirect_surface_message(
> + p, sfid, brw_writemask(brw_null_reg(), mask),
> + payload, surface, msg_length, 0, align1);
> +
> + brw_set_dp_untyped_surface_write_message(
> + p, insn, num_channels);
> +}
> +
> void
> brw_pixel_interpolator_query(struct brw_compile *p,
> struct brw_reg dest,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 6e6ce58..6d363f1 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -508,6 +508,7 @@ fs_inst::is_send_from_grf() const
> c
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
On Thu, Mar 12, 2015 at 07:25:34PM +0200, Francisco Jerez wrote:
> "Pohjolainen, Topi" writes:
>
> > On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> >> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> >> surface index as a register instead of a constant and to use
> >> brw_send_indirect_message() to emit the indirect variant of send with
> >> a dynamically calculated message descriptor. This will be required to
> >> support variable indexing of image arrays for
> >> ARB_shader_image_load_store.
> >> ---
> >> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> >> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
> >> +--
> >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
> >> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
> >> 4 files changed, 96 insertions(+), 80 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> >> b/src/mesa/drivers/dri/i965/brw_eu.h
> >> index 87a9f3f..9cc9123 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
> >>
> >> void
> >> brw_untyped_atomic(struct brw_compile *p,
> >> - struct brw_reg dest,
> >> + struct brw_reg dst,
> >> struct brw_reg payload,
> >> + struct brw_reg surface,
> >> unsigned atomic_op,
> >> - unsigned bind_table_index,
> >> unsigned msg_length,
> >> bool response_expected);
> >>
> >> void
> >> brw_untyped_surface_read(struct brw_compile *p,
> >> - struct brw_reg dest,
> >> - struct brw_reg mrf,
> >> - unsigned bind_table_index,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg surface,
> >> unsigned msg_length,
> >> unsigned num_channels);
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> index 0b655d4..34695bf 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
> >> return setup;
> >> }
> >>
> >> +static struct brw_inst *
> >> +brw_send_indirect_surface_message(struct brw_compile *p,
> >> + unsigned sfid,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg surface,
> >> + unsigned message_len,
> >> + unsigned response_len,
> >> + bool header_present)
> >> +{
> >> + const struct brw_context *brw = p->brw;
> >> + struct brw_inst *insn;
> >> +
> >> + if (surface.file != BRW_IMMEDIATE_VALUE) {
> >> + struct brw_reg addr = retype(brw_address_reg(0),
> >> BRW_REGISTER_TYPE_UD);
> >> +
> >> + brw_push_insn_state(p);
> >> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> >> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> >> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> >> +
> >> + /* Mask out invalid bits from the surface index to avoid hangs e.g.
> >> when
> >> + * some surface array is accessed out of bounds.
> >> + */
> >> + insn = brw_AND(p, addr,
> >> + suboffset(vec1(retype(surface,
> >> BRW_REGISTER_TYPE_UD)),
> >> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
> >> + brw_imm_ud(0xff));
> >> +
> >> + brw_pop_insn_state(p);
> >> +
> >> + surface = addr;
> >
> > Also this patch didn't really need this branch at all. Instead it could
> > have been its own patch and here just:
> >
> > assert(surface.file == BRW_IMMEDIATE_VALUE);
> >
> The whole point of this patch is to fix untyped surface message sends to
> deal with non-immediate "surface" arguments. How could it not be
> needed?
I mean that the patch could be split in two parts: one doing the
re-organization and another adding the non-immediate branch to
brw_send_indirect_surface_message().
If I'm reading the patch right, brw_untyped_surface_read() and
brw_untyped_atomic() are only called with surface indices of immediate type.
Hence the added branch is not yet hit in runtime at this point.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
"Pohjolainen, Topi" writes:
> On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
>> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
>> surface index as a register instead of a constant and to use
>> brw_send_indirect_message() to emit the indirect variant of send with
>> a dynamically calculated message descriptor. This will be required to
>> support variable indexing of image arrays for
>> ARB_shader_image_load_store.
>> ---
>> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
>> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
>> +--
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
>> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
>> 4 files changed, 96 insertions(+), 80 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
>> b/src/mesa/drivers/dri/i965/brw_eu.h
>> index 87a9f3f..9cc9123 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
>>
>> void
>> brw_untyped_atomic(struct brw_compile *p,
>> - struct brw_reg dest,
>> + struct brw_reg dst,
>> struct brw_reg payload,
>> + struct brw_reg surface,
>> unsigned atomic_op,
>> - unsigned bind_table_index,
>> unsigned msg_length,
>> bool response_expected);
>>
>> void
>> brw_untyped_surface_read(struct brw_compile *p,
>> - struct brw_reg dest,
>> - struct brw_reg mrf,
>> - unsigned bind_table_index,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg surface,
>> unsigned msg_length,
>> unsigned num_channels);
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> index 0b655d4..34695bf 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
>> return setup;
>> }
>>
>> +static struct brw_inst *
>> +brw_send_indirect_surface_message(struct brw_compile *p,
>> + unsigned sfid,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg surface,
>> + unsigned message_len,
>> + unsigned response_len,
>> + bool header_present)
>> +{
>> + const struct brw_context *brw = p->brw;
>> + struct brw_inst *insn;
>> +
>> + if (surface.file != BRW_IMMEDIATE_VALUE) {
>> + struct brw_reg addr = retype(brw_address_reg(0),
>> BRW_REGISTER_TYPE_UD);
>> +
>> + brw_push_insn_state(p);
>> + brw_set_default_access_mode(p, BRW_ALIGN_1);
>> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
>> +
>> + /* Mask out invalid bits from the surface index to avoid hangs e.g.
>> when
>> + * some surface array is accessed out of bounds.
>> + */
>> + insn = brw_AND(p, addr,
>> + suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
>> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
>> + brw_imm_ud(0xff));
>> +
>> + brw_pop_insn_state(p);
>> +
>> + surface = addr;
>
> Also this patch didn't really need this branch at all. Instead it could
> have been its own patch and here just:
>
> assert(surface.file == BRW_IMMEDIATE_VALUE);
>
The whole point of this patch is to fix untyped surface message sends to
deal with non-immediate "surface" arguments. How could it not be
needed?
>> + }
>> +
>> + insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
>> + brw_inst_set_mlen(brw, insn, message_len);
>> + brw_inst_set_rlen(brw, insn, response_len);
>> + brw_inst_set_header_present(brw, insn, header_present);
>> +
>> + return insn;
>> +}
>> +
>> static int
>> brw_find_next_block_end(struct brw_compile *p, int start_offset)
>> {
>> @@ -2747,25 +2789,16 @@ static void
>> brw_set_dp_untyped_atomic_message(struct brw_compile *p,
>>brw_inst *insn,
>>unsigned atomic_op,
>> - unsigned bind_table_index,
>> - unsigned msg_length,
>> - unsigned response_length,
>> - bool header_present)
>> + bool response_expected)
>> {
>> const struct brw_c
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
"Pohjolainen, Topi" writes:
> On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
>> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
>> surface index as a register instead of a constant and to use
>> brw_send_indirect_message() to emit the indirect variant of send with
>> a dynamically calculated message descriptor. This will be required to
>> support variable indexing of image arrays for
>> ARB_shader_image_load_store.
>> ---
>> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
>> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
>> +--
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
>> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
>> 4 files changed, 96 insertions(+), 80 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
>> b/src/mesa/drivers/dri/i965/brw_eu.h
>> index 87a9f3f..9cc9123 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
>>
>> void
>> brw_untyped_atomic(struct brw_compile *p,
>> - struct brw_reg dest,
>> + struct brw_reg dst,
>> struct brw_reg payload,
>> + struct brw_reg surface,
>> unsigned atomic_op,
>> - unsigned bind_table_index,
>> unsigned msg_length,
>> bool response_expected);
>>
>> void
>> brw_untyped_surface_read(struct brw_compile *p,
>> - struct brw_reg dest,
>> - struct brw_reg mrf,
>> - unsigned bind_table_index,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg surface,
>> unsigned msg_length,
>> unsigned num_channels);
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> index 0b655d4..34695bf 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
>> return setup;
>> }
>>
>> +static struct brw_inst *
>> +brw_send_indirect_surface_message(struct brw_compile *p,
>> + unsigned sfid,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg surface,
>> + unsigned message_len,
>> + unsigned response_len,
>> + bool header_present)
>> +{
>> + const struct brw_context *brw = p->brw;
>> + struct brw_inst *insn;
>> +
>> + if (surface.file != BRW_IMMEDIATE_VALUE) {
>> + struct brw_reg addr = retype(brw_address_reg(0),
>> BRW_REGISTER_TYPE_UD);
>> +
>> + brw_push_insn_state(p);
>> + brw_set_default_access_mode(p, BRW_ALIGN_1);
>> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
>> +
>> + /* Mask out invalid bits from the surface index to avoid hangs e.g.
>> when
>> + * some surface array is accessed out of bounds.
>> + */
>> + insn = brw_AND(p, addr,
>> + suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
>> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
>> + brw_imm_ud(0xff));
>> +
>> + brw_pop_insn_state(p);
>> +
>> + surface = addr;
>> + }
>> +
>> + insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
>> + brw_inst_set_mlen(brw, insn, message_len);
>> + brw_inst_set_rlen(brw, insn, response_len);
>> + brw_inst_set_header_present(brw, insn, header_present);
>> +
>> + return insn;
>> +}
>> +
>> static int
>> brw_find_next_block_end(struct brw_compile *p, int start_offset)
>> {
>> @@ -2747,25 +2789,16 @@ static void
>> brw_set_dp_untyped_atomic_message(struct brw_compile *p,
>>brw_inst *insn,
>>unsigned atomic_op,
>> - unsigned bind_table_index,
>> - unsigned msg_length,
>> - unsigned response_length,
>> - bool header_present)
>> + bool response_expected)
>> {
>> const struct brw_context *brw = p->brw;
>> -
>> unsigned msg_control =
>>atomic_op | /* Atomic Operation Type: BRW_AOP_* */
>> - (response_length ? 1 << 5 : 0); /* Return data expected */
>> + (response_expected ? 1 << 5 : 0); /* Return data expected */
>>
>> if (brw->gen >= 8 || brw->is_haswell) {
>> - br
Re: [Mesa-dev] [PATCH 04/13] i965: Mask out unused Align16 components in brw_untyped_atomic.
On Fri, Mar 06, 2015 at 02:56:24PM +0200, Francisco Jerez wrote: > "Pohjolainen, Topi" writes: > > > On Fri, Feb 27, 2015 at 05:34:47PM +0200, Francisco Jerez wrote: > >> This is currently not a problem because the vec4 visitor happens to > >> mask out unused components from the destination, but it might become > >> an issue when we start using atomics without writeback message. In > >> any case it seems sensible to set it again here because the > >> consequences of setting the wrong writemask (random graphics memory > >> corruption) are difficult to debug and can easily go unnoticed. > > > > I started thinking if this should be an assertion here and should we force > > the logic in the visitor to consider the writemask correctly instead? I > > don't > > have a strong opinion, merely just wondering aloud. > > > > That would be rather inconvenient for my (not yet sent for review) > ARB_shader_image_load_store intrinsic lowering code. If we made it an > assertion, say: > > | emit(some_surface_opcode, vgrf(rlen), payload, surface, control); > > or > > | emit(some_surface_opcode, reg_null_ud(), payload, surface, control); > > would fail with an assertion failure. I need that to just work no > matter what surface opcode is specified. > > It would also be somewhat misleading, because these messages have the > annoying property of clobbering destination register components even if > they're masked out, so in Align16 they kind of always behave as if > WRITEMASK_XYZW was specified as far as the destination region is > concerned. Just sent a reply with few nits. But otherwise I'm fine with the patch: Reviewed-by: Topi Pohjolainen ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4 v3] mesa: Separate PBO validation checks from buffer mapping, to allow reuse
On Thu, Mar 12, 2015 at 12:14 AM, Eduardo Lima Mitev
wrote:
> Internal PBO functions such as _mesa_map_validate_pbo_source() and
> _mesa_validate_pbo_compressed_teximage() perform validation and buffer
> mapping
> within the same call.
>
> This patch takes out the validation into separate functions to allow reuse
> of functionality by other code (i.e, gl(Compressed)Tex(Sub)Image).
> ---
> src/mesa/main/pbo.c | 117
> ++--
> src/mesa/main/pbo.h | 14 +++
> 2 files changed, 100 insertions(+), 31 deletions(-)
>
> diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
> index 259f763..85c32a1 100644
> --- a/src/mesa/main/pbo.c
> +++ b/src/mesa/main/pbo.c
> @@ -164,23 +164,18 @@ _mesa_map_pbo_source(struct gl_context *ctx,
> return buf;
> }
>
> -
> /**
> - * Combine PBO-read validation and mapping.
> - * If any GL errors are detected, they'll be recorded and NULL returned.
> + * Perform PBO validation for read operations with uncompressed textures.
> + * If any GL errors are detected, false is returned, otherwise returns
> true.
> * \sa _mesa_validate_pbo_access
> - * \sa _mesa_map_pbo_source
> - * A call to this function should have a matching call to
> - * _mesa_unmap_pbo_source().
> */
> -const GLvoid *
> -_mesa_map_validate_pbo_source(struct gl_context *ctx,
> - GLuint dimensions,
> - const struct gl_pixelstore_attrib *unpack,
> - GLsizei width, GLsizei height, GLsizei
> depth,
> - GLenum format, GLenum type,
> - GLsizei clientMemSize,
> - const GLvoid *ptr, const char *where)
> +bool
> +_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,
> + const struct gl_pixelstore_attrib *unpack,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type,
> + GLsizei clientMemSize,
> + const GLvoid *ptr, const char *where)
> {
> assert(dimensions == 1 || dimensions == 2 || dimensions == 3);
>
> @@ -188,24 +183,85 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,
>format, type, clientMemSize, ptr)) {
>if (_mesa_is_bufferobj(unpack->BufferObj)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
>
Add the "PBO" back to this comment.
> - "%s(out of bounds PBO access)", where);
> + "%s(out of bounds access)",
> + where);
>} else {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> "%s(out of bounds access: bufSize (%d) is too
> small)",
> where, clientMemSize);
>}
> - return NULL;
> + return false;
> }
>
> if (!_mesa_is_bufferobj(unpack->BufferObj)) {
>/* non-PBO access: no further validation to be done */
> - return ptr;
> + return true;
> }
>
> if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
>/* buffer is already mapped - that's an error */
> - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", where);
> - return NULL;
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
> + where);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * Perform PBO validation for read operations with compressed textures.
> + * If any GL errors are detected, false is returned, otherwise returns
> true.
> + */
> +bool
> +_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint
> dimensions,
> + const struct gl_pixelstore_attrib
> *unpack,
> + GLsizei imageSize, const GLvoid
> *pixels,
> + const char *where)
> +{
> + if (!_mesa_is_bufferobj(unpack->BufferObj)) {
> + /* not using a PBO */
> + return true;
> + }
> +
> + if ((const GLubyte *) pixels + imageSize >
> + ((const GLubyte *) 0) + unpack->BufferObj->Size) {
> + /* out of bounds read! */
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid PBO access)",
> + where);
> + return false;
> + }
> +
> + if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
> + /* buffer is already mapped - that's an error */
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
> + where);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * Perform PBO-read mapping.
> + * If any GL errors are detected, they'll be recorded and NULL returned.
> + * \sa _mesa_validate_pbo_source
> + * \sa _mesa_map_pbo_source
> + * A call to this function should have a matching call to
> + * _mesa_unmap_pbo_source().
> + */
> +const GLvoid *
> +_mesa_map_validate_pbo_source(struct gl_c
Re: [Mesa-dev] [PATCH 04/13] i965: Mask out unused Align16 components in brw_untyped_atomic.
On Fri, Feb 27, 2015 at 05:34:47PM +0200, Francisco Jerez wrote:
> This is currently not a problem because the vec4 visitor happens to
> mask out unused components from the destination, but it might become
> an issue when we start using atomics without writeback message. In
> any case it seems sensible to set it again here because the
> consequences of setting the wrong writemask (random graphics memory
> corruption) are difficult to debug and can easily go unnoticed.
> ---
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 2b1d6ff..0b655d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2799,16 +2799,25 @@ brw_untyped_atomic(struct brw_compile *p,
> bool response_expected)
> {
> const struct brw_context *brw = p->brw;
> + const bool align1 = (brw_inst_access_mode(brw, p->current) ==
> BRW_ALIGN_1);
Most of the code in the backend does not use the extra ().
> + /* Mask out unused components -- This is especially important in Align16
> +* mode on generations that don't have native support for SIMD4x2 atomics,
> +* because unused but enabled components will cause the dataport to
> perform
> +* additional atomic operations on the addresses that happen to be in the
> +* uninitialized Y, Z and W coordinates of the payload.
> +*/
> + const unsigned mask = (align1 ? WRITEMASK_XYZW : WRITEMASK_X);
Same here.
> brw_inst *insn = brw_next_insn(p, BRW_OPCODE_SEND);
>
> - brw_set_dest(p, insn, retype(dest, BRW_REGISTER_TYPE_UD));
> + brw_set_dest(p, insn, retype(brw_writemask(dest, mask),
> +BRW_REGISTER_TYPE_UD));
> brw_set_src0(p, insn, retype(payload, BRW_REGISTER_TYPE_UD));
> brw_set_src1(p, insn, brw_imm_d(0));
> brw_set_dp_untyped_atomic_message(
>p, insn, atomic_op, bind_table_index, msg_length,
>brw_surface_payload_size(p, response_expected,
> brw->gen >= 8 || brw->is_haswell, true),
> - brw_inst_access_mode(brw, insn) == BRW_ALIGN_1);
> + align1);
> }
>
> static void
> --
> 2.1.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> surface index as a register instead of a constant and to use
> brw_send_indirect_message() to emit the indirect variant of send with
> a dynamically calculated message descriptor. This will be required to
> support variable indexing of image arrays for
> ARB_shader_image_load_store.
> ---
> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
> +--
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
> 4 files changed, 96 insertions(+), 80 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 87a9f3f..9cc9123 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
>
> void
> brw_untyped_atomic(struct brw_compile *p,
> - struct brw_reg dest,
> + struct brw_reg dst,
> struct brw_reg payload,
> + struct brw_reg surface,
> unsigned atomic_op,
> - unsigned bind_table_index,
> unsigned msg_length,
> bool response_expected);
>
> void
> brw_untyped_surface_read(struct brw_compile *p,
> - struct brw_reg dest,
> - struct brw_reg mrf,
> - unsigned bind_table_index,
> + struct brw_reg dst,
> + struct brw_reg payload,
> + struct brw_reg surface,
> unsigned msg_length,
> unsigned num_channels);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 0b655d4..34695bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
> return setup;
> }
>
> +static struct brw_inst *
> +brw_send_indirect_surface_message(struct brw_compile *p,
> + unsigned sfid,
> + struct brw_reg dst,
> + struct brw_reg payload,
> + struct brw_reg surface,
> + unsigned message_len,
> + unsigned response_len,
> + bool header_present)
> +{
> + const struct brw_context *brw = p->brw;
> + struct brw_inst *insn;
> +
> + if (surface.file != BRW_IMMEDIATE_VALUE) {
> + struct brw_reg addr = retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> +
> + brw_push_insn_state(p);
> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> +
> + /* Mask out invalid bits from the surface index to avoid hangs e.g.
> when
> + * some surface array is accessed out of bounds.
> + */
> + insn = brw_AND(p, addr,
> + suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
> + brw_imm_ud(0xff));
> +
> + brw_pop_insn_state(p);
> +
> + surface = addr;
Also this patch didn't really need this branch at all. Instead it could
have been its own patch and here just:
assert(surface.file == BRW_IMMEDIATE_VALUE);
> + }
> +
> + insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
> + brw_inst_set_mlen(brw, insn, message_len);
> + brw_inst_set_rlen(brw, insn, response_len);
> + brw_inst_set_header_present(brw, insn, header_present);
> +
> + return insn;
> +}
> +
> static int
> brw_find_next_block_end(struct brw_compile *p, int start_offset)
> {
> @@ -2747,25 +2789,16 @@ static void
> brw_set_dp_untyped_atomic_message(struct brw_compile *p,
>brw_inst *insn,
>unsigned atomic_op,
> - unsigned bind_table_index,
> - unsigned msg_length,
> - unsigned response_length,
> - bool header_present)
> + bool response_expected)
> {
> const struct brw_context *brw = p->brw;
> -
> unsigned msg_control =
>atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> - (response_length ? 1 << 5 : 0); /* Return data expected */
> + (response_expected ? 1 << 5 : 0); /* Return data expected */
>
> if (brw->gen >= 8 || brw->is_haswell) {
Re: [Mesa-dev] New stable-branch 10.5 candidate pushed
Emil, I spoke with Neil Roberts, and the two patches of mine you have listed as rejected here are required basis for his four queued commits (all of these pertain to meta_pbo_texSubImage). The only patch that was reverted was my "1D ARRAY" patch. Laura On Thu, Mar 12, 2015 at 6:50 AM, Emil Velikov wrote: > Hello list, > > The candidate for the Mesa 10.5.1 is now available. The current patch queue > is as follows: > - 57 queued > - 3 nominated (outstanding) > - and 2 rejected (obsolete) patches > > This provides us with over a dozen i965 fixes for all over the driver, some > r300g ones (correct handling of RGTC1 and LATC1 formats, fixed sRGB blits) > and improvements in the freedreno compiler along side with fixed rendering > in Xonotic and others. > > Take a look at section "Mesa stable queue" for more information. > > Testing > --- > The following results are against piglit 305ecc3ac89. > > > Changes - classic i965(snb) > --- > None. > > > Changes - swrast classic > > None. > > > Changes - gallium softpipe, llvmpipe (LLVM 3.5.1) > > None. > > > Testing reports/general approval > > Any testing reports (or general approval of the state of the branch) > will be greatly appreciated. > > > Trivial merge conflicts > --- > Here are the commits where I manually merged conflicts, so these might > merit additional review: > > commit 31fcb21ef523434a254c0bbff515345c2c6d8152 > Author: Matt Turner > > i965: Avoid applying negate to wrong MAD source. > > (cherry picked from commit d528907fd2950c7bb968fff66dd79863cd128890) > > commit c3fc8b2870668fe0313fd35b2789306dbf3b9594 > Author: Kenneth Graunke > > i965/fs: Set force_writemask_all on shader_time instructions. > > (cherry picked from commit ef9cc7d0c176669c03130abf576f2b700be39514) > > commit 82ef4994ddc041b101bcda8e729e729d93b0 > Author: Kenneth Graunke > > i965/fs: Set smear on shader_time diff register. > > (cherry picked from commit f1adc45dbe649cdd4538fb96f6d2a27328bbfba1) > > > commit c232d765affc06cc6e81ddee07656919e7f17aa5 > Author: Kenneth Graunke > > i965/fs: Make emit_shader_time_end() insert before EOT. > > (cherry picked from commit 4ebeb71573ad44f7657810dc5dd2c9030e3e63db) > > > commit fbd06fe65c0fe57f0dea96c87d9f0eb5abc72bb7 > Author: Kenneth Graunke > > i965/fs: Don't issue FB writes for bound but unwritten color targets. > > (cherry picked from commit e95969cd9548033250ba12f2adf11740319b41e7) > > > The plan is to have 10.5.1 this Friday(13th March). > > If you have any questions or comments that you would like to share > before the release, please go ahead. > > > Cheers, > Emil > > > Mesa stable queue > - > > Nominated (3) > = > > Anuj Phogat (1): > glsl: Generate link error for non-matching gl_FragCoord > redeclarations > > Brian Paul (1): > configure: don't try to build gallium DRI drivers if --disable-dri > is set > > Mario Kleiner (1): > glx: Handle out-of-sequence swap completion events correctly. > > > Rejected (2) > > > Laura Ekstrand (2): > common: Correct texture init for meta pbo uploads and downloads. > common: Correct PBO 2D_ARRAY handling. > > > Queued (57) > === > > Andrey Sudnik (1): > i965/vec4: Don't lose the saturate modifier in copy propagation. > > Chris Forbes (1): > i965/gs: Check newly-generated GS-out VUE map against correct stage > > Daniel Stone (1): > egl: Take alpha bits into account when selecting GBM formats > > Emil Velikov (4): > docs: Add sha256 sums for the 10.5.0 release > egl/main: no longer export internal function > cherry-ignore: ignore a few more commits picked without -x > mapi: fix commit 90411b56f6bc817e229d8801ac0adad6d4e3fb7a > > Frank Henigman (1): > intel: fix EGLImage renderbuffer _BaseFormat > > Iago Toral Quiroga (1): > i965: Fix out-of-bounds accesses into pull_constant_loc array > > Ian Romanick (1): > i965/fs/nir: Use emit_math for nir_op_fpow > > Ilia Mirkin (3): > freedreno: move fb state copy after checking for size change > freedreno/ir3: fix array count returned by TXQ > freedreno/ir3: get the # of miplevels from getinfo > > Jason Ekstrand (2): > meta/TexSubImage: Stash everything other than PIXEL_TRANSFER/store > in meta_begin > main/base_tex_format: Properly handle STENCIL_INDEX1/4/16 > > Kenneth Graunke (8): > i965: Split Gen4-5 BlitFramebuffer code; prefer BLT over Meta. > glsl: Mark array access when copying to a temporary for the ?: > operator. > i965/fs: Set force_writemask_all on shader_time instructions. > i965/fs: Set smear on shader_time diff register. > i965/fs: Make emit_shader_time_write return rather than emit. > i965/fs: Make get_timestamp() pass back the MOV rather than emittin
Re: [Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> surface index as a register instead of a constant and to use
> brw_send_indirect_message() to emit the indirect variant of send with
> a dynamically calculated message descriptor. This will be required to
> support variable indexing of image arrays for
> ARB_shader_image_load_store.
> ---
> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158
> +--
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
> 4 files changed, 96 insertions(+), 80 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 87a9f3f..9cc9123 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
>
> void
> brw_untyped_atomic(struct brw_compile *p,
> - struct brw_reg dest,
> + struct brw_reg dst,
> struct brw_reg payload,
> + struct brw_reg surface,
> unsigned atomic_op,
> - unsigned bind_table_index,
> unsigned msg_length,
> bool response_expected);
>
> void
> brw_untyped_surface_read(struct brw_compile *p,
> - struct brw_reg dest,
> - struct brw_reg mrf,
> - unsigned bind_table_index,
> + struct brw_reg dst,
> + struct brw_reg payload,
> + struct brw_reg surface,
> unsigned msg_length,
> unsigned num_channels);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index 0b655d4..34695bf 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
> return setup;
> }
>
> +static struct brw_inst *
> +brw_send_indirect_surface_message(struct brw_compile *p,
> + unsigned sfid,
> + struct brw_reg dst,
> + struct brw_reg payload,
> + struct brw_reg surface,
> + unsigned message_len,
> + unsigned response_len,
> + bool header_present)
> +{
> + const struct brw_context *brw = p->brw;
> + struct brw_inst *insn;
> +
> + if (surface.file != BRW_IMMEDIATE_VALUE) {
> + struct brw_reg addr = retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> +
> + brw_push_insn_state(p);
> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> +
> + /* Mask out invalid bits from the surface index to avoid hangs e.g.
> when
> + * some surface array is accessed out of bounds.
> + */
> + insn = brw_AND(p, addr,
> + suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
> + brw_imm_ud(0xff));
> +
> + brw_pop_insn_state(p);
> +
> + surface = addr;
> + }
> +
> + insn = brw_send_indirect_message(p, sfid, dst, payload, surface);
> + brw_inst_set_mlen(brw, insn, message_len);
> + brw_inst_set_rlen(brw, insn, response_len);
> + brw_inst_set_header_present(brw, insn, header_present);
> +
> + return insn;
> +}
> +
> static int
> brw_find_next_block_end(struct brw_compile *p, int start_offset)
> {
> @@ -2747,25 +2789,16 @@ static void
> brw_set_dp_untyped_atomic_message(struct brw_compile *p,
>brw_inst *insn,
>unsigned atomic_op,
> - unsigned bind_table_index,
> - unsigned msg_length,
> - unsigned response_length,
> - bool header_present)
> + bool response_expected)
> {
> const struct brw_context *brw = p->brw;
> -
> unsigned msg_control =
>atomic_op | /* Atomic Operation Type: BRW_AOP_* */
> - (response_length ? 1 << 5 : 0); /* Return data expected */
> + (response_expected ? 1 << 5 : 0); /* Return data expected */
>
> if (brw->gen >= 8 || brw->is_haswell) {
> - brw_set_message_descriptor(p, insn, HSW_SFID_DATAPORT_DATA_CACHE_1,
> - msg_length, response_length,
> -
[Mesa-dev] [Bug 89562] glXSwapIntervalMESA crashes when making use of GLX_MESA_multithread_makecurrent
https://bugs.freedesktop.org/show_bug.cgi?id=89562 Bug ID: 89562 Summary: glXSwapIntervalMESA crashes when making use of GLX_MESA_multithread_makecurrent Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: GLX Assignee: mesa-dev@lists.freedesktop.org Reporter: jl...@feralinteractive.com QA Contact: mesa-dev@lists.freedesktop.org Created attachment 114264 --> https://bugs.freedesktop.org/attachment.cgi?id=114264&action=edit test case If two threads have the same current glX context, as permitted by GLX_MESA_multithread_makecurrent, then calling glXSwapIntervalMESA results in invoking dri2SetSwapInterval with the pdraw argument set to NULL. dri2SetSwapInterval then dereferences pdraw, causing a segmentation fault. The attached program reproduces this. You can also reproduce the same crash by letting a thread exit with a context bound and then making the context current on another thread and calling glXSwapIntervalMESA there (this seems a bit dirty, but if there is no requirement to unbind GL contexts before exiting threads, then this method doesn't require GLX_MESA_multithread_makecurrent). If you add -DTEST2 to the compiler flags when compiling the attached file, this crash will be reproduced instead. This affects at least Mesa 10.4.3, 10.5, and git master at revision 48b0a3c1c9d829a9b1d401afb2796b35df94a5d7. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] i965: Factor out logic to build a send message instruction with indirect descriptor.
On Thu, Mar 12, 2015 at 7:10 AM, Francisco Jerez wrote: > Meh, I've modified PATCH 01 so it drops the send destination type > changes in the Gen7 pull constant load code (see attachment). > > Do we know if the destination type of the SEND instruction has any other > subtle effects, like, affecting the computation of dependency scoreboard > signals? At least none that I'm aware of. :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] nir: Optimize a + neg(a)
On Sat, Feb 28, 2015 at 9:19 PM, Matt Turner wrote: > On Sat, Feb 28, 2015 at 11:47 AM, Thomas Helland > wrote: >> On Feb 28, 2015 8:39 PM, "Jason Ekstrand" wrote: >>> >>> Both patches are >>> >>> Reviewed-by: Jason Ekstrand >> >> Could you commit them? >> I don't have commit access. > > I'd like to wait a few days to see if there are any comments about the > floating-point optimizations. If not, I'll commit them. (The integer > parts are surely fine) "a + - a" should give NaN if a is NaN or INF, not zero. However, the GLSL spec says "NaNs are not required to be generated" and "Operations and built-in functions that operate on a NaN are not required to return a NaN as the result". So I think the patch is good. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
Michel Dänzer wrote on 12.03.2015 08:15: > On 11.03.2015 05:07, Vivek Dasmohapatra wrote: >> Hi - As you probably already know, there can only be one version of >> libstdc++.so in your runtime link chain - This is usually not a problem, >> but when things are linked against the Steam runtime (for example), they >> can end up with two - one from the steam runtime, and one pulled in via >> the mesa dri libs from the OS/distribution. > > How can that happen? The problems I've seen related to this were usually > because Steam overrides the system libstdc++ / libgcc with older > versions, which breaks other system libraries. > > Can somebody please tell Valve that doing that is not okay? And it's not > necessary either. Each library can be compared to the system version > individually and only overridden when necessary, e.g. by putting each > library into its own directory and only adding the necessary directories > to $LD_LIBRARY_PATH. E.g. VMware use this approach in their products. This. Though I'd actually like Valve to go even farther and at least start creating dummy dependency packages for things they install. This way, they can ensure all dependencies are met and they only have to ship closed-source stuff. Bonus side-effect: only one copy of a library present, which gets updated by the central package manager. If they use something like PackageKit they shouldn't even have too much problems with the DEB/RPM split. Since you could generate these dependencies automatically – SteamOS (aka Debian 7.x) is their base after all, anything has to run there – that wouldn't constitute too much of a maintenance burden either. (Obviously: true packages with a full build service behind it, would be best, but I don't expect that to ever happen.) But I guess everybody could be (mostly) happy with what Michel suggested. And in addition to what has been said before: I wouldn't expect such a change to take root in any (sane) distribution. So this change either turns into work for the distributions (reverting the change) or the relevant configure option will always be set to "disable". Cheers, Kai signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: move fpclassify work-arounds into c99_math.h
On 03/12/2015 04:41 AM, Jose Fonseca wrote:
On 11/03/15 16:48, Brian Paul wrote:
v2: Use #error in the #else clause, per Jose.
---
include/c99_math.h | 44 ++
src/mesa/main/querymatrix.c | 51
+
2 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/include/c99_math.h b/include/c99_math.h
index 0a49950..bd35d1b 100644
--- a/include/c99_math.h
+++ b/include/c99_math.h
@@ -161,4 +161,48 @@ llrintf(float f)
#endif
+#if defined(fpclassify)
+/* ISO C99 says that fpclassify is a macro. Assume that any
implementation
+ * of fpclassify, whether it's in a C99 compiler or not, will be a
macro.
+ */
+#elif defined(__cplusplus)
+/* For C++, fpclassify() should be defined in */
+#elif defined(_MSC_VER)
+/* Not required on VS2013 and above. Oddly, the fpclassify() function
+ * doesn't exist in such a form on MSVC. This is an implementation
using
+ * slightly different lower-level Windows functions.
+ */
+#include
+
+static inline enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL,
FP_NORMAL}
+fpclassify(double x)
+{
+ switch(_fpclass(x)) {
+ case _FPCLASS_SNAN: /* signaling NaN */
+ case _FPCLASS_QNAN: /* quiet NaN */
+ return FP_NAN;
+ case _FPCLASS_NINF: /* negative infinity */
+ case _FPCLASS_PINF: /* positive infinity */
+ return FP_INFINITE;
+ case _FPCLASS_NN: /* negative normal */
+ case _FPCLASS_PN: /* positive normal */
+ return FP_NORMAL;
+ case _FPCLASS_ND: /* negative denormalized */
+ case _FPCLASS_PD: /* positive denormalized */
+ return FP_SUBNORMAL;
+ case _FPCLASS_NZ: /* negative zero */
+ case _FPCLASS_PZ: /* positive zero */
+ return FP_ZERO;
+ default:
+ /* Should never get here; but if we do, this will guarantee
+ * that the pattern is not treated like a number.
+ */
+ return FP_NAN;
+ }
+}
+#else
+#error "Need to include or define an fpclassify function"
+#endif
+
+
#endif /* #define _C99_MATH_H_ */
diff --git a/src/mesa/main/querymatrix.c b/src/mesa/main/querymatrix.c
index ef85175..095817c 100644
--- a/src/mesa/main/querymatrix.c
+++ b/src/mesa/main/querymatrix.c
@@ -13,7 +13,7 @@
#include
-#include
+#include "c99_math.h"
#include "glheader.h"
#include "querymatrix.h"
#include "main/get.h"
@@ -37,55 +37,6 @@
#define INT_TO_FIXED(x) ((GLfixed) ((x) << 16))
#define FLOAT_TO_FIXED(x) ((GLfixed) ((x) * 65536.0))
-#if defined(fpclassify)
-/* ISO C99 says that fpclassify is a macro. Assume that any
implementation
- * of fpclassify, whether it's in a C99 compiler or not, will be a
macro.
- */
-#elif defined(_MSC_VER)
-/* Not required on VS2013 and above. */
-/* Oddly, the fpclassify() function doesn't exist in such a form
- * on MSVC. This is an implementation using slightly different
- * lower-level Windows functions.
- */
-#include
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
-switch(_fpclass(x)) {
-case _FPCLASS_SNAN: /* signaling NaN */
-case _FPCLASS_QNAN: /* quiet NaN */
-return FP_NAN;
-case _FPCLASS_NINF: /* negative infinity */
-case _FPCLASS_PINF: /* positive infinity */
-return FP_INFINITE;
-case _FPCLASS_NN: /* negative normal */
-case _FPCLASS_PN: /* positive normal */
-return FP_NORMAL;
-case _FPCLASS_ND: /* negative denormalized */
-case _FPCLASS_PD: /* positive denormalized */
-return FP_SUBNORMAL;
-case _FPCLASS_NZ: /* negative zero */
-case _FPCLASS_PZ: /* positive zero */
-return FP_ZERO;
-default:
-/* Should never get here; but if we do, this will guarantee
- * that the pattern is not treated like a number.
- */
-return FP_NAN;
-}
-}
-
-#else
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
- /* XXX do something better someday */
- return FP_NORMAL;
-}
-
-#endif
GLbitfield GLAPIENTRY _mesa_QueryMatrixxOES(GLfixed mantissa[16],
GLint exponent[16])
{
Reviewed-by: Jose Fonseca
Thanks. Pushed. If this causes any other problems I'm tempted to
simply nuke GL_OES_query_matrix and the _mesa_QueryMatrixxOES() function
which is the only user of fpclassify(). I've never seen an app that
uses this extension and we don't have any piglit tests for it.
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: fix cast to silence compiler warning
eglcurrent.c: In function '_eglSetTSD':
eglcurrent.c:57:4: warning: passing argument 2 of 'tss_set' discards
'const' qualifier from pointer target type [enabled by default]
tss_set(_egl_TSD, (const void *) t);
^
In file included from ../../../include/c11/threads.h:72:0,
from eglcurrent.c:32:
../../../include/c11/threads_posix.h:357:1: note: expected 'void *'
but argument is of type 'const void *'
tss_set(tss_t key, void *val)
^
---
src/egl/main/eglcurrent.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/egl/main/eglcurrent.c b/src/egl/main/eglcurrent.c
index 5d8cae4..6ffc799 100644
--- a/src/egl/main/eglcurrent.c
+++ b/src/egl/main/eglcurrent.c
@@ -54,7 +54,7 @@ static __thread const _EGLThreadInfo *_egl_TLS
static inline void _eglSetTSD(const _EGLThreadInfo *t)
{
- tss_set(_egl_TSD, (const void *) t);
+ tss_set(_egl_TSD, (void *) t);
#ifdef GLX_USE_TLS
_egl_TLS = t;
#endif
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] i965: Factor out logic to build a send message instruction with indirect descriptor.
Francisco Jerez writes:
> Matt Turner writes:
>
>> On Wed, Mar 11, 2015 at 2:29 PM, Francisco Jerez
>> wrote:
>>> Matt Turner writes:
commit 4c4934636cb286e7d7836afc26e9d392e2f0f155
Author: Paul Berry
Date: Tue Sep 24 15:18:52 2013 -0700
i965/blorp: retype destination register for texture SEND instruction
to UW.
The resource streamer only exists on HSW+, so the UW dest is certainly
needed for things after Gen5.
>>>
>>> Odd, I haven't seen a mention of that restriction in the hardware specs
>>> (at least not on reasonably recent ones -- the Gen4 and 5 specs do
>>> mention it and they actually hang if you send a message with compression
>>> enabled and anything bigger than a W as destination type). Is this a
>>> purely empirical finding? If so, doesn't it deserve a big fat warning
>>> comment? Is this only a problem for some interaction with the resource
>>> streamer or has it ever been observed to fix something else?
>>
>> This page [0] says:
>>
>> """
>> The subregister number, horizontal stride, destination mask and type
>> fields of are always valid and are used in part to generate the
>> WrEn. This is true even if is a null register (this is an
>> exception for null as for most cases these fields are ignored by
>> hardware). These parameters of follow the same restriction as
>> that of normal destination operand – destination region cannot cross
>> the 256-bit register boundary.
>> """
>>
>> Searching for the exact phrase quoted in Paul's commit finds another
>> page that says it applies to "DevSNB+,Pre-DevBDW".
>>
>
> Hm, searching for the same phrase ("destination region cannot cross the
> 256-bit register boundary.") gives several matches, some of the pages
> are indeed tagged "DevSNB+,Pre-DevBDW", but in all of them that phrase
> appears inside a SNB-only block, I don't see any indication of that
> restriction applying to Gen7 and up.
>
Meh, I've modified PATCH 01 so it drops the send destination type
changes in the Gen7 pull constant load code (see attachment).
Do we know if the destination type of the SEND instruction has any other
subtle effects, like, affecting the computation of dependency scoreboard
signals?
>> I think this is one of those cases where they technically give you all
>> the information, they just don't tell you anything about what you're
>> supposed to do with it. Totally bullshit, but par for the course.
>>
>> I guess the good news in all of this is that we now know we don't need
>> to bother with this for BDW+.
>>
>> [0] 3D-Media-GPGPU Engine > EU Overview > ISA Introduction >
>> Instruction Set Reference > EUISA Instructions > Send Message [SNB+]
From 592a35f79ecfac77d0aee33ff134c4f0c73b95f7 Mon Sep 17 00:00:00 2001
From: Francisco Jerez
Date: Mon, 9 Mar 2015 19:33:10 +0200
Subject: [PATCH] i965: Factor out logic to build a send message instruction
with indirect descriptor.
This is going to be useful because the Gen7+ uniform and varying pull
constant, texturing, typed and untyped surface read, write, and atomic
generation code on the vec4 and fs back-end all require the same logic
to handle conditionally indirect surface indices. In pseudocode:
| if (surface.file == BRW_IMMEDIATE_VALUE) {
| inst = brw_SEND(p, dst, payload);
| set_descriptor_control_bits(inst, surface, ...);
| } else {
| inst = brw_OR(p, addr, surface, 0);
| set_descriptor_control_bits(inst, ...);
| inst = brw_SEND(p, dst, payload);
| set_indirect_send_descriptor(inst, addr);
| }
This patch abstracts out this frequently recurring pattern so we can
now write:
| inst = brw_send_indirect_message(p, sfid, dst, payload, surface)
| set_descriptor_control_bits(inst, ...);
without worrying about handling the immediate and indirect surface
index cases explicitly.
v2: Rebase. Improve documentatation and commit message. (Topi)
Preserve UW destination type cargo-cult. (Topi, Ken, Matt)
Reviewed-by: Topi Pohjolainen
Acked-by: Kenneth Graunke
---
src/mesa/drivers/dri/i965/brw_eu.h | 21 ++--
src/mesa/drivers/dri/i965/brw_eu_emit.c | 58 --
src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 63 ++--
src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 37 +++---
4 files changed, 83 insertions(+), 96 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
index 736c54b..e2b5b75 100644
--- a/src/mesa/drivers/dri/i965/brw_eu.h
+++ b/src/mesa/drivers/dri/i965/brw_eu.h
@@ -205,11 +205,6 @@ void brw_set_sampler_message(struct brw_compile *p,
unsigned simd_mode,
unsigned return_format);
-void brw_set_indirect_send_descriptor(struct brw_compile *p,
- brw_inst *insn,
- unsigned sfid,
- struct brw_reg descriptor);
-
void brw_set_
[Mesa-dev] [Bug 89553] Account request
https://bugs.freedesktop.org/show_bug.cgi?id=89553 Brian Paul changed: What|Removed |Added Component|Mesa core |New Accounts Assignee|mesa-dev@lists.freedesktop. |sitewranglers@lists.freedes |org |ktop.org Product|Mesa|freedesktop.org QA Contact|mesa-dev@lists.freedesktop. | |org | --- Comment #2 from Brian Paul --- Fine by me. Reassigning to admins. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] New stable-branch 10.5 candidate pushed
Hello list, The candidate for the Mesa 10.5.1 is now available. The current patch queue is as follows: - 57 queued - 3 nominated (outstanding) - and 2 rejected (obsolete) patches This provides us with over a dozen i965 fixes for all over the driver, some r300g ones (correct handling of RGTC1 and LATC1 formats, fixed sRGB blits) and improvements in the freedreno compiler along side with fixed rendering in Xonotic and others. Take a look at section "Mesa stable queue" for more information. Testing --- The following results are against piglit 305ecc3ac89. Changes - classic i965(snb) --- None. Changes - swrast classic None. Changes - gallium softpipe, llvmpipe (LLVM 3.5.1) None. Testing reports/general approval Any testing reports (or general approval of the state of the branch) will be greatly appreciated. Trivial merge conflicts --- Here are the commits where I manually merged conflicts, so these might merit additional review: commit 31fcb21ef523434a254c0bbff515345c2c6d8152 Author: Matt Turner i965: Avoid applying negate to wrong MAD source. (cherry picked from commit d528907fd2950c7bb968fff66dd79863cd128890) commit c3fc8b2870668fe0313fd35b2789306dbf3b9594 Author: Kenneth Graunke i965/fs: Set force_writemask_all on shader_time instructions. (cherry picked from commit ef9cc7d0c176669c03130abf576f2b700be39514) commit 82ef4994ddc041b101bcda8e729e729d93b0 Author: Kenneth Graunke i965/fs: Set smear on shader_time diff register. (cherry picked from commit f1adc45dbe649cdd4538fb96f6d2a27328bbfba1) commit c232d765affc06cc6e81ddee07656919e7f17aa5 Author: Kenneth Graunke i965/fs: Make emit_shader_time_end() insert before EOT. (cherry picked from commit 4ebeb71573ad44f7657810dc5dd2c9030e3e63db) commit fbd06fe65c0fe57f0dea96c87d9f0eb5abc72bb7 Author: Kenneth Graunke i965/fs: Don't issue FB writes for bound but unwritten color targets. (cherry picked from commit e95969cd9548033250ba12f2adf11740319b41e7) The plan is to have 10.5.1 this Friday(13th March). If you have any questions or comments that you would like to share before the release, please go ahead. Cheers, Emil Mesa stable queue - Nominated (3) = Anuj Phogat (1): glsl: Generate link error for non-matching gl_FragCoord redeclarations Brian Paul (1): configure: don't try to build gallium DRI drivers if --disable-dri is set Mario Kleiner (1): glx: Handle out-of-sequence swap completion events correctly. Rejected (2) Laura Ekstrand (2): common: Correct texture init for meta pbo uploads and downloads. common: Correct PBO 2D_ARRAY handling. Queued (57) === Andrey Sudnik (1): i965/vec4: Don't lose the saturate modifier in copy propagation. Chris Forbes (1): i965/gs: Check newly-generated GS-out VUE map against correct stage Daniel Stone (1): egl: Take alpha bits into account when selecting GBM formats Emil Velikov (4): docs: Add sha256 sums for the 10.5.0 release egl/main: no longer export internal function cherry-ignore: ignore a few more commits picked without -x mapi: fix commit 90411b56f6bc817e229d8801ac0adad6d4e3fb7a Frank Henigman (1): intel: fix EGLImage renderbuffer _BaseFormat Iago Toral Quiroga (1): i965: Fix out-of-bounds accesses into pull_constant_loc array Ian Romanick (1): i965/fs/nir: Use emit_math for nir_op_fpow Ilia Mirkin (3): freedreno: move fb state copy after checking for size change freedreno/ir3: fix array count returned by TXQ freedreno/ir3: get the # of miplevels from getinfo Jason Ekstrand (2): meta/TexSubImage: Stash everything other than PIXEL_TRANSFER/store in meta_begin main/base_tex_format: Properly handle STENCIL_INDEX1/4/16 Kenneth Graunke (8): i965: Split Gen4-5 BlitFramebuffer code; prefer BLT over Meta. glsl: Mark array access when copying to a temporary for the ?: operator. i965/fs: Set force_writemask_all on shader_time instructions. i965/fs: Set smear on shader_time diff register. i965/fs: Make emit_shader_time_write return rather than emit. i965/fs: Make get_timestamp() pass back the MOV rather than emitting it. i965/fs: Make emit_shader_time_end() insert before EOT. i965/fs: Don't issue FB writes for bound but unwritten color targets. Laura Ekstrand (2): main: Fix target checking for CompressedTexSubImage*D. main: Fix target checking for CopyTexSubImage*D. Marc-Andre Lureau (1): gallium/auxiliary/indices: fix start param Marek Olšák (3): r300g: fix RGTC1 and LATC1 SNORM formats r300g: fix a crash when resolving into an sRGB texture r300g: fix sRGB->sRGB blits Matt Turner (12): i965/vec4: Fix implementation of i2b. m
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
"Pohjolainen, Topi" writes:
> On Tue, Mar 10, 2015 at 11:07:26PM +0200, Francisco Jerez wrote:
>> "Pohjolainen, Topi" writes:
>>
>> > On Mon, Mar 09, 2015 at 12:43:08PM +0200, Francisco Jerez wrote:
>> >> "Pohjolainen, Topi" writes:
>> >>
>> >> > On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote:
>> >> >> Topi Pohjolainen writes:
>> >> >>
>> >> >> > The original patch from Curro was based on something that is not
>> >> >> > present in the master yet. This patch tries to mimick the logic on
>> >> >> > top master.
>> >> >> > I wanted to see if could separate the building of the descriptor
>> >> >> > instruction from building of the send instruction. This logic now
>> >> >> > allows the caller to construct any kind of sequence of instructions
>> >> >> > filling in the descriptor before giving it to the send instruction
>> >> >> > builder.
>> >> >> >
>> >> >> > This is only compile tested. Curro, how would you feel about this
>> >> >> > sort of approach? I apologise for muddying the waters again but I
>> >> >> > wasn't entirely comfortable with the logic and wanted to try to
>> >> >> > something else.
>> >> >> >
>> >> >> > I believe patch number five should go nicely on top of this as
>> >> >> > the descriptor instruction could be followed by (or preceeeded by)
>> >> >> > any additional instructions modifying the descriptor register
>> >> >> > before the actual send instruction.
>> >> >> >
>> >> >>
>> >> >> Topi, the goal I had in mind with PATCH 01 was to refactor a commonly
>> >> >> recurring pattern. In terms of the functions defined in this patch my
>> >> >> example from yesterday [1] would now look like:
>> >> >>
>> >> >> | if (index.file == BRW_IMMEDIATE_VALUE) {
>> >> >> |
>> >> >> | uint32_t surf_index = index.dw1.ud;
>> >> >> |
>> >> >> | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
>> >> >> | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW));
>> >> >> | brw_set_src0(p, send, offset);
>> >> >> | brw_set_sampler_message(p, send,
>> >> >> | surf_index,
>> >> >> | 0, /* LD message ignores sampler unit */
>> >> >> | GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
>> >> >> | rlen,
>> >> >> | mlen,
>> >> >> | false, /* no header */
>> >> >> | simd_mode,
>> >> >> | 0);
>> >> >> |
>> >> >> | brw_mark_surface_used(prog_data, surf_index);
>> >> >> |
>> >> >> | } else {
>> >> >> |
>> >> >> | struct brw_reg addr = vec1(retype(brw_address_reg(0),
>> >> >> BRW_REGISTER_TYPE_UD));
>> >> >> |
>> >> >> | brw_push_insn_state(p);
>> >> >> | brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> >> >> | brw_set_default_access_mode(p, BRW_ALIGN_1);
>> >> >> |
>> >> >> | /* a0.0 = surf_index & 0xff */
>> >> >> | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND);
>> >> >> | brw_inst_set_exec_size(p->brw, insn_and, BRW_EXECUTE_1);
>> >> >> | brw_set_dest(p, insn_and, addr);
>> >> >> | brw_set_src0(p, insn_and, vec1(retype(index,
>> >> >> BRW_REGISTER_TYPE_UD)));
>> >> >> | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff));
>> >> >> |
>> >> >> |
>> >> >> | /* a0.0 |= */
>> >> >> | brw_inst *descr_inst = brw_build_indirect_message_descr(p,
>> >> >> addr, addr);
>> >> >> | brw_set_sampler_message(p, descr_inst,
>> >> >> | 0 /* surface */,
>> >> >> | 0 /* sampler */,
>> >> >> | GEN5_SAMPLER_MESSAGE_SAMPLE_LD,
>> >> >> | rlen /* rlen */,
>> >> >> | mlen /* mlen */,
>> >> >> | false /* header */,
>> >> >> | simd_mode,
>> >> >> | 0);
>> >> >> |
>> >> >> | /* dst = send(offset, a0.0) */
>> >> >> | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset,
>> >> >> addr);
>> >> >> |
>> >> >> | brw_pop_insn_state(p);
>> >> >> |
>> >> >> | /* visitor knows more than we do about the surface limit
>> >> >> required,
>> >> >> | * so has already done marking.
>> >> >> | */
>> >> >> | }
>> >> >
>> >> > Which I think could also be written as follows. Or am I missing
>> >> > something
>> >> > again?
>> >> >
>> >> > static brw_inst *
>> >> > brw_build_surface_index_descr(struct brw_compile *p,
>> >> > struct brw_reg dst, index)
>> >> > {
>> >> >brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> >> >brw_set_default_access_mode(p, BRW_ALIGN_1);
>> >> >
>> >> >/* a0.0 = surf_index & 0xff */
>> >> >brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND);
>> >> >brw_inst_set_exec_size(p->brw, insn_and, BRW_EXECUTE_1);
>> >> >brw_set_dest(p, insn_and, addr);
>> >> >brw_set_src0
Re: [Mesa-dev] [PATCH] mesa: move fpclassify work-arounds into c99_math.h
On 11/03/15 16:48, Brian Paul wrote:
v2: Use #error in the #else clause, per Jose.
---
include/c99_math.h | 44 ++
src/mesa/main/querymatrix.c | 51 +
2 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/include/c99_math.h b/include/c99_math.h
index 0a49950..bd35d1b 100644
--- a/include/c99_math.h
+++ b/include/c99_math.h
@@ -161,4 +161,48 @@ llrintf(float f)
#endif
+#if defined(fpclassify)
+/* ISO C99 says that fpclassify is a macro. Assume that any implementation
+ * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
+ */
+#elif defined(__cplusplus)
+/* For C++, fpclassify() should be defined in */
+#elif defined(_MSC_VER)
+/* Not required on VS2013 and above. Oddly, the fpclassify() function
+ * doesn't exist in such a form on MSVC. This is an implementation using
+ * slightly different lower-level Windows functions.
+ */
+#include
+
+static inline enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
+fpclassify(double x)
+{
+ switch(_fpclass(x)) {
+ case _FPCLASS_SNAN: /* signaling NaN */
+ case _FPCLASS_QNAN: /* quiet NaN */
+ return FP_NAN;
+ case _FPCLASS_NINF: /* negative infinity */
+ case _FPCLASS_PINF: /* positive infinity */
+ return FP_INFINITE;
+ case _FPCLASS_NN: /* negative normal */
+ case _FPCLASS_PN: /* positive normal */
+ return FP_NORMAL;
+ case _FPCLASS_ND: /* negative denormalized */
+ case _FPCLASS_PD: /* positive denormalized */
+ return FP_SUBNORMAL;
+ case _FPCLASS_NZ: /* negative zero */
+ case _FPCLASS_PZ: /* positive zero */
+ return FP_ZERO;
+ default:
+ /* Should never get here; but if we do, this will guarantee
+ * that the pattern is not treated like a number.
+ */
+ return FP_NAN;
+ }
+}
+#else
+#error "Need to include or define an fpclassify function"
+#endif
+
+
#endif /* #define _C99_MATH_H_ */
diff --git a/src/mesa/main/querymatrix.c b/src/mesa/main/querymatrix.c
index ef85175..095817c 100644
--- a/src/mesa/main/querymatrix.c
+++ b/src/mesa/main/querymatrix.c
@@ -13,7 +13,7 @@
#include
-#include
+#include "c99_math.h"
#include "glheader.h"
#include "querymatrix.h"
#include "main/get.h"
@@ -37,55 +37,6 @@
#define INT_TO_FIXED(x) ((GLfixed) ((x) << 16))
#define FLOAT_TO_FIXED(x) ((GLfixed) ((x) * 65536.0))
-#if defined(fpclassify)
-/* ISO C99 says that fpclassify is a macro. Assume that any implementation
- * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
- */
-#elif defined(_MSC_VER)
-/* Not required on VS2013 and above. */
-/* Oddly, the fpclassify() function doesn't exist in such a form
- * on MSVC. This is an implementation using slightly different
- * lower-level Windows functions.
- */
-#include
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
-switch(_fpclass(x)) {
-case _FPCLASS_SNAN: /* signaling NaN */
-case _FPCLASS_QNAN: /* quiet NaN */
-return FP_NAN;
-case _FPCLASS_NINF: /* negative infinity */
-case _FPCLASS_PINF: /* positive infinity */
-return FP_INFINITE;
-case _FPCLASS_NN: /* negative normal */
-case _FPCLASS_PN: /* positive normal */
-return FP_NORMAL;
-case _FPCLASS_ND: /* negative denormalized */
-case _FPCLASS_PD: /* positive denormalized */
-return FP_SUBNORMAL;
-case _FPCLASS_NZ: /* negative zero */
-case _FPCLASS_PZ: /* positive zero */
-return FP_ZERO;
-default:
-/* Should never get here; but if we do, this will guarantee
- * that the pattern is not treated like a number.
- */
-return FP_NAN;
-}
-}
-
-#else
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
- /* XXX do something better someday */
- return FP_NORMAL;
-}
-
-#endif
GLbitfield GLAPIENTRY _mesa_QueryMatrixxOES(GLfixed mantissa[16], GLint
exponent[16])
{
Reviewed-by: Jose Fonseca
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/6] c11: add c11 compatibility wrapper around stdlib.h
On 12/03/15 00:07, Emil Velikov wrote: On 9 March 2015 at 11:54, Jose Fonseca wrote: On 07/03/15 19:38, Emil Velikov wrote: On 07/03/15 07:23, Jose Fonseca wrote: ... we still didn't eliminate the use of non-portable _MTX_INITIALIZER_NP from Mesa tree gave me pause. The only way I can think about resolving this, is to use call_once() to initialize the mutex, Yes, I'm afraid so. Which makes me wonder, why does no other place in mesa (be that glx, gallium etc.) do so already ? I believe mostly for convenience, as I said. I was the one who introduced c11 threads emulation headers, and replacing the non-portable static initialization with call_once would be a lot of work which I wasn't ready to take on at that time, hence I introduced _MTX_INITIALIZER_NP to facilite the transition. I was also hopeful another revision of ISO C would introduce static initializer in threads. But nowadays I suspect it might be not. Also it seem that some places might have to stick to pthreads. Haven't checked the details though. * src/glx/apple pthread_threadid_np pthread_is_threaded_np * src/gallium/auxiliary/os pthread_sigmask pthread_setname_np Yeah, I think it's unavoidable. I think that it's fair to assume that when C11 threads are implemented by the system, they can interoperate with pthreads, even though the c11 threads types are not mere typedefs of pthread types. If worst come true, we can always stick with our custom c11_threads.h implementation. * src/gallium/state_trackers/omx/ pthread_join ? missing a matching thread_create Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89387] Double delete in lp_bld_misc.cpp
https://bugs.freedesktop.org/show_bug.cgi?id=89387 José Fonseca changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from José Fonseca --- (In reply to Chris Vine from comment #2) > That will work fine. Thanks. > This does beg the question whether the code is correct for llvm < 3.6 (it > fixes llvm >= 3.6). I believe so. LLVM 3.4 and 3.3's documentation for setMCJITMemoryManager/setJITMemoryManager state they only take onwership if builder.create() is successful. And indeed we only delete MM when builder.create() fails. Fixed on http://cgit.freedesktop.org/mesa/mesa/commit/?id=70dc8a9930f561d7ce6db7e58b5bc9b4d940e37b -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.
On Wed, Mar 11, 2015 at 2:09 PM, Ilia Mirkin wrote: > On Wed, Mar 11, 2015 at 5:57 PM, wrote: >> From: Marius Predut > > Set your email from name correctly in git and then you won't have this > line in your git send-email results. > >> >> On SNB and IVB hw, for 1 pixel line thickness or less, >> the general anti-aliasing algorithm give up - garbage line is generated. >> Setting a Line Width of 0.0 specifies the rasterization >> of the “thinnest” (one-pixel-wide), non-antialiased lines. >> Lines rendered with zero Line Width are rasterized using >> Grid Intersection Quantization rules as specified by >> bspec section 6.3.12.1 Zero-Width (Cosmetic) Line Rasterization. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668 > > This seems like the wrong bug reference... I'm pretty sure it should be https://bugs.freedesktop.org/show_bug.cgi?id=28832 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] glsl: Expose built-in packing functions under GLSL 4.2.
Thanks. I noticed this too, but was too busy to fix it.
For the series:
Reviewed-by: Marek Olšák
Marek
On Thu, Mar 12, 2015 at 2:45 AM, Matt Turner wrote:
> ARB_shading_language_packing is part of GLSL 4.2, not 4.0 as I
> mistakenly believed. The following functions are available only with
> ARB_shading_language_packing, GLSL 4.2 (not GLSL 4.0), or ES 3.0:
>
>- packSnorm2x16
>- unpackSnorm2x16
>- packHalf2x16
>- unpackHalf2x16
> ---
> src/glsl/builtin_functions.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index 84bbdc2..c607572 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -201,7 +201,7 @@ static bool
> shader_packing_or_es3(const _mesa_glsl_parse_state *state)
> {
> return state->ARB_shading_language_packing_enable ||
> - state->is_version(400, 300);
> + state->is_version(420, 300);
> }
>
> static bool
> --
> 2.0.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] i965: Factor out logic to build a send message instruction with indirect descriptor.
On Wed, Mar 11, 2015 at 1:07 PM, Kenneth Graunke wrote: > On Wednesday, March 11, 2015 07:25:14 PM Francisco Jerez wrote: >> "Pohjolainen, Topi" writes: >> > On Fri, Feb 27, 2015 at 05:34:44PM +0200, Francisco Jerez wrote: >> >> @@ -1218,17 +1198,6 @@ >> >> fs_generator::generate_varying_pull_constant_load_gen7(fs_inst *inst, >> >>false /* header */, >> >>simd_mode, >> >>0); >> >> - brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1); >> >> - brw_inst_set_src1_reg_type(p->brw, insn_or, BRW_REGISTER_TYPE_UD); >> >> - brw_set_src0(p, insn_or, addr); >> >> - brw_set_dest(p, insn_or, addr); >> >> - >> >> - >> >> - /* dst = send(offset, a0.0) */ >> >> - brw_inst *insn_send = brw_next_insn(p, BRW_OPCODE_SEND); >> >> - brw_set_dest(p, insn_send, retype(dst, BRW_REGISTER_TYPE_UW)); >> > >> > I'm just reading this through again and noticed that the destination type >> > changes here to BRW_REGISTER_TYPE_UD (set in brw_send_indirect_message()). >> >> Ah, yes, that change is intentional. The type being set to UW was a >> remnant from Gen4-5 times -- Those used to require the destination type >> of SEND to be W/UW when doing 16-wide (even if the message was actually >> writing dwords back...). None of the codepaths modified in this patch >> (or in the rest of the series) should be executed on Gen4 or 5. >> >> Anyway good catch. :) > > I'm pretty sure this does still apply on Gen7 - notably, BLORP uses UW > destinations for SIMD16 sends, and I believe Paul Berry tracked actual > issues down to that. BLORP only exists on Gen6+. > > I also seem to recall Chris Forbes hitting an issue relating to that > which he and Paul fixed during XDC 2013. > > CC'ing Chris and Matt in case they remember any details. I believe he was helping Abdiel track down a hang when using the resource streamer, and it turned out that using UW typed-destinations on sends in blorp fixed it: commit 4c4934636cb286e7d7836afc26e9d392e2f0f155 Author: Paul Berry Date: Tue Sep 24 15:18:52 2013 -0700 i965/blorp: retype destination register for texture SEND instruction to UW. The resource streamer only exists on HSW+, so the UW dest is certainly needed for things after Gen5. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] i965/fs: Store a pointer to brw_sampler_prog_key_data in the visitor.
On Wednesday, March 11, 2015 10:44:26 AM Pohjolainen, Topi wrote: > On Mon, Mar 09, 2015 at 01:58:51AM -0700, Kenneth Graunke wrote: > > The NIR backend hardcodes brw_wm_prog_key at the moment, which won't > > work when we support scalar VS. We could use get_tex(), but it's a > > static method. I was going to promote it to fs_visitor, but then > > realized that both parameters (stage and key) are already members. > > > > It then occured to me that we could just set up a pointer in the > > constructor, and skip having a function altogether. > > > > This patch also converts all existing users to use key_tex. > > > > Signed-off-by: Kenneth Graunke > > With the few nits: > > Reviewed-by: Topi Pohjolainen > > > --- > > src/mesa/drivers/dri/i965/brw_fs.h | 2 + > > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +- > > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 55 > > > > 3 files changed, 27 insertions(+), 33 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.h > > b/src/mesa/drivers/dri/i965/brw_fs.h > > index ee6ba98..7f4916a 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs.h > > +++ b/src/mesa/drivers/dri/i965/brw_fs.h > > @@ -418,6 +418,8 @@ public: > > void visit_atomic_counter_intrinsic(ir_call *ir); > > > > const void *const key; > > + struct brw_sampler_prog_key_data *key_tex; > > This could be constant pointer as well, we only use it for reading in the > visitor. (Also prevents the need to cast the constant 'key' pointer to > non-constant in init()). Good catch! I've changed it to: const struct brw_sampler_prog_key_Data *key_tex; and changed the casts in init() to be "const struct brw_wm_prog_key" and so on. I couldn't add the second 'const' (which prevents assignments to key_tex) since I can't really set this from a constructor initializer list - I need the switch statement in init(). But, this is probably the most important one. Also fixed the word wrapping. Thanks! signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] i965/vs: Add missing resolve_bool_comparison calls on GEN4 and GEN5
On 03/11/2015 10:44 PM, Ian Romanick wrote: From: Ian Romanick The ir_unop_any problem was discovered by some later optimization passes that generate ir_triop_csel. I was also able to reproduce it by modifying the gl-2.0-vertexattribpointer vertex shader to generate its result using color = mix(vec4(0, 1, 0, 0), vec4(1, 0, 0, 0), bvec4(any(greaterThan(diff, vec4(tolerance); instead of an if-statement. This also required using #version 130 and MESA_GLSL_VERSION_OVERRIDE=130. I have not nominated this for stable releases because I don't think there's any way to trigger the problem without GLSL 1.30 or optimizations that don't exist in stable. Signed-off-by: Ian Romanick Patch 1 is Reviewed-by: Abdiel Janulgue ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/13] i965: Factor out logic to build a send message instruction with indirect descriptor.
"Pohjolainen, Topi" writes:
> On Fri, Feb 27, 2015 at 05:34:44PM +0200, Francisco Jerez wrote:
>> ---
>> src/mesa/drivers/dri/i965/brw_eu.h | 19 ++--
>> src/mesa/drivers/dri/i965/brw_eu_emit.c | 58
>> ++--
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 55 +-
>> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 37 ---
>> 4 files changed, 77 insertions(+), 92 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
>> b/src/mesa/drivers/dri/i965/brw_eu.h
>> index 1b954c8..9b1e0e2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu.h
>> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
>> @@ -205,11 +205,6 @@ void brw_set_sampler_message(struct brw_compile *p,
>> unsigned simd_mode,
>> unsigned return_format);
>>
>> -void brw_set_indirect_send_descriptor(struct brw_compile *p,
>> - brw_inst *insn,
>> - unsigned sfid,
>> - struct brw_reg descriptor);
>> -
>> void brw_set_dp_read_message(struct brw_compile *p,
>> brw_inst *insn,
>> unsigned binding_table_index,
>> @@ -243,6 +238,20 @@ void brw_urb_WRITE(struct brw_compile *p,
>> unsigned offset,
>> unsigned swizzle);
>>
>> +/**
>> + * Send message to shared unit \p sfid with a possibly indirect descriptor
>> \p
>> + * desc. If the descriptor is not an immediate it will be transparently
>> + * loaded to an address register using an OR instruction that will be
>> returned
>> + * to the caller so additional descriptor bits can be specified with the
>> usual
>> + * brw_set_*_message() helper functions.
>> + */
>> +struct brw_inst *
>> +brw_send_indirect_message(struct brw_compile *p,
>> + unsigned sfid,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg desc);
>> +
>> void brw_ff_sync(struct brw_compile *p,
>> struct brw_reg dest,
>> unsigned msg_reg_nr,
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> index e69840a..cd2ce92 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>> @@ -751,21 +751,6 @@ brw_set_sampler_message(struct brw_compile *p,
>> }
>> }
>>
>> -void brw_set_indirect_send_descriptor(struct brw_compile *p,
>> - brw_inst *insn,
>> - unsigned sfid,
>> - struct brw_reg descriptor)
>> -{
>> - /* Only a0.0 may be used as SEND's descriptor operand. */
>> - assert(descriptor.file == BRW_ARCHITECTURE_REGISTER_FILE);
>> - assert(descriptor.type == BRW_REGISTER_TYPE_UD);
>> - assert(descriptor.nr == BRW_ARF_ADDRESS);
>> - assert(descriptor.subnr == 0);
>> -
>> - brw_set_message_descriptor(p, insn, sfid, 0, 0, false, false);
>> - brw_set_src1(p, insn, descriptor);
>> -}
>> -
>> static void
>> gen7_set_dp_scratch_message(struct brw_compile *p,
>> brw_inst *inst,
>> @@ -2490,6 +2475,49 @@ void brw_urb_WRITE(struct brw_compile *p,
>> swizzle);
>> }
>>
>> +struct brw_inst *
>> +brw_send_indirect_message(struct brw_compile *p,
>> + unsigned sfid,
>> + struct brw_reg dst,
>> + struct brw_reg payload,
>> + struct brw_reg desc)
>> +{
>> + const struct brw_context *brw = p->brw;
>> + struct brw_inst *send, *setup;
>> +
>> + assert(desc.type == BRW_REGISTER_TYPE_UD);
>> +
>> + if (desc.file == BRW_IMMEDIATE_VALUE) {
>> + setup = send = next_insn(p, BRW_OPCODE_SEND);
>> + brw_set_src1(p, send, desc);
>> +
>> + } else {
>> + struct brw_reg addr = retype(brw_address_reg(0),
>> BRW_REGISTER_TYPE_UD);
>> +
>> + brw_push_insn_state(p);
>> + brw_set_default_access_mode(p, BRW_ALIGN_1);
>> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
>> +
>> + /* Load the indirect descriptor to an address register using OR so the
>> + * caller can specify additional descriptor bits with the usual
>> + * brw_set_*_message() helper functions.
>> + */
>> + setup = brw_OR(p, addr, desc, brw_imm_ud(0));
>> +
>> + brw_pop_insn_state(p);
>> +
>> + send = next_insn(p, BRW_OPCODE_SEND);
>> + brw_set_src1(p, send, addr);
>> + }
>> +
>> + brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UD));
>> + brw_set_src0(p, send, retype(payload, BRW_REGISTER_TYPE_UD));
>> + brw_inst_set_sfid(brw, send, sfid);
>> +
>> + return setup;
>> +}
>> +
>> static
Re: [Mesa-dev] [PATCH] i965: Fix out-of-bounds accesses into pull_constant_loc array
Given the age of this bug, should this be a candidate for 10.4 and 10.5 stable branches? On 03/10/2015 03:36 AM, Iago Toral Quiroga wrote: > The piglit test glsl-fs-uniform-array-loop-unroll.shader_test was designed > to do an out of bounds access into an uniform array to make sure that we > handle that situation gracefully inside the driver, however, as Ken describes > in bug 79202, Valgrind reports that this is leading to an out-of-bounds access > in fs_visitor::demote_pull_constants(). > > Before accessing the pull_constant_loc array we should make sure that > the uniform we are trying to access is valid. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79202 > --- > src/mesa/drivers/dri/i965/brw_fs.cpp | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > b/src/mesa/drivers/dri/i965/brw_fs.cpp > index 89754ad..6d7cf0e 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > @@ -2281,8 +2281,13 @@ fs_visitor::demote_pull_constants() >if (inst->src[i].file != UNIFORM) > continue; > > - int pull_index = pull_constant_loc[inst->src[i].reg + > -inst->src[i].reg_offset]; > + int pull_index; > + unsigned location = inst->src[i].reg + inst->src[i].reg_offset; > + if (location >= uniforms) /* Out of bounds access */ > +pull_index = -1; > + else > +pull_index = pull_constant_loc[location]; > + > if (pull_index == -1) > continue; > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
Ian Romanick writes: > On 03/11/2015 09:31 AM, Tobias Klausmann wrote: >> The problem in not forcing this to link statically is, that if a >> distribution decides to not use this static option, the problem persists >> on that distribution. On top every lib pulled in by steam from the >> system would need to be link statically, like mesa. Instead of fixing >> every lib steam pulls in (how many are there?), fix the steam runtime to > > Yeah, static linking is a terrible, partial solution. > >> a) not ship libstdc++.so and libgcc_s.so and declare older version of >> these libs as not supported (thats what people do when they face the >> incompatibility problem with steams versions of these libs: just delete >> them from the runtime and everything is fine) > > Let's be 100% clear. This is NOT an option for Steam. They ship > thousands of closed-source applications. These applications were built > and tested against specific versions of specific libraries. Removing > support for old run-times is equivalent to removing support for those > applications. They can't tell their customers, "You upgraded your > distro, so that game you paid money for no longer works. Tough break, kid." > > Forcing an application to use a newer run-time may work over the short > term, but do you think that will still work in 5 years? It's possible, > but it seems unlikely. And "seems to work" is not the same as "works > all the way through the whole game every time." That's why the GNU C++ runtime uses library and symbol versioning to allow ABI-incompatible versions to coexist. It would be possible for Steam to provide a "reference" version of the C++ runtime that would be used in cases where either the operating system version is too old (in which case other libraries from the OS linking to a libstdc++ with same SONAME are guaranteed to be forwards-compatible with Steam's), or in cases where the operating system version is so new that it's ABI-incompatible with Steam's (in which case it would be possible for both versions to coexist thanks to symbol versioning so pulling libraries from the OS wouldn't pose a problem). In all other cases they ought to be using the C++ runtime from the operating system if they don't want to distribute their own libGL... signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: move fpclassify work-arounds into c99_math.h
v2: Use #error in the #else clause, per Jose.
---
include/c99_math.h | 44 ++
src/mesa/main/querymatrix.c | 51 +
2 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/include/c99_math.h b/include/c99_math.h
index 0a49950..bd35d1b 100644
--- a/include/c99_math.h
+++ b/include/c99_math.h
@@ -161,4 +161,48 @@ llrintf(float f)
#endif
+#if defined(fpclassify)
+/* ISO C99 says that fpclassify is a macro. Assume that any implementation
+ * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
+ */
+#elif defined(__cplusplus)
+/* For C++, fpclassify() should be defined in */
+#elif defined(_MSC_VER)
+/* Not required on VS2013 and above. Oddly, the fpclassify() function
+ * doesn't exist in such a form on MSVC. This is an implementation using
+ * slightly different lower-level Windows functions.
+ */
+#include
+
+static inline enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
+fpclassify(double x)
+{
+ switch(_fpclass(x)) {
+ case _FPCLASS_SNAN: /* signaling NaN */
+ case _FPCLASS_QNAN: /* quiet NaN */
+ return FP_NAN;
+ case _FPCLASS_NINF: /* negative infinity */
+ case _FPCLASS_PINF: /* positive infinity */
+ return FP_INFINITE;
+ case _FPCLASS_NN: /* negative normal */
+ case _FPCLASS_PN: /* positive normal */
+ return FP_NORMAL;
+ case _FPCLASS_ND: /* negative denormalized */
+ case _FPCLASS_PD: /* positive denormalized */
+ return FP_SUBNORMAL;
+ case _FPCLASS_NZ: /* negative zero */
+ case _FPCLASS_PZ: /* positive zero */
+ return FP_ZERO;
+ default:
+ /* Should never get here; but if we do, this will guarantee
+ * that the pattern is not treated like a number.
+ */
+ return FP_NAN;
+ }
+}
+#else
+#error "Need to include or define an fpclassify function"
+#endif
+
+
#endif /* #define _C99_MATH_H_ */
diff --git a/src/mesa/main/querymatrix.c b/src/mesa/main/querymatrix.c
index ef85175..095817c 100644
--- a/src/mesa/main/querymatrix.c
+++ b/src/mesa/main/querymatrix.c
@@ -13,7 +13,7 @@
#include
-#include
+#include "c99_math.h"
#include "glheader.h"
#include "querymatrix.h"
#include "main/get.h"
@@ -37,55 +37,6 @@
#define INT_TO_FIXED(x) ((GLfixed) ((x) << 16))
#define FLOAT_TO_FIXED(x) ((GLfixed) ((x) * 65536.0))
-#if defined(fpclassify)
-/* ISO C99 says that fpclassify is a macro. Assume that any implementation
- * of fpclassify, whether it's in a C99 compiler or not, will be a macro.
- */
-#elif defined(_MSC_VER)
-/* Not required on VS2013 and above. */
-/* Oddly, the fpclassify() function doesn't exist in such a form
- * on MSVC. This is an implementation using slightly different
- * lower-level Windows functions.
- */
-#include
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
-switch(_fpclass(x)) {
-case _FPCLASS_SNAN: /* signaling NaN */
-case _FPCLASS_QNAN: /* quiet NaN */
-return FP_NAN;
-case _FPCLASS_NINF: /* negative infinity */
-case _FPCLASS_PINF: /* positive infinity */
-return FP_INFINITE;
-case _FPCLASS_NN: /* negative normal */
-case _FPCLASS_PN: /* positive normal */
-return FP_NORMAL;
-case _FPCLASS_ND: /* negative denormalized */
-case _FPCLASS_PD: /* positive denormalized */
-return FP_SUBNORMAL;
-case _FPCLASS_NZ: /* negative zero */
-case _FPCLASS_PZ: /* positive zero */
-return FP_ZERO;
-default:
-/* Should never get here; but if we do, this will guarantee
- * that the pattern is not treated like a number.
- */
-return FP_NAN;
-}
-}
-
-#else
-
-enum {FP_NAN, FP_INFINITE, FP_ZERO, FP_SUBNORMAL, FP_NORMAL}
-fpclassify(double x)
-{
- /* XXX do something better someday */
- return FP_NORMAL;
-}
-
-#endif
GLbitfield GLAPIENTRY _mesa_QueryMatrixxOES(GLfixed mantissa[16], GLint
exponent[16])
{
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
On Wed, Mar 11, 2015 at 12:40 PM, Ian Romanick wrote: > On 03/11/2015 09:31 AM, Tobias Klausmann wrote: >> The problem in not forcing this to link statically is, that if a >> distribution decides to not use this static option, the problem persists >> on that distribution. On top every lib pulled in by steam from the >> system would need to be link statically, like mesa. Instead of fixing >> every lib steam pulls in (how many are there?), fix the steam runtime to > > Yeah, static linking is a terrible, partial solution. Actually seems like it's the only reasonable solution... For a closed-source application provider who doesn't want to do builds against every conceivable combination of their dependencies, they just need to be moved into the binary and be done with it. (Although LGPL might prevent that, not sure what the various licenses say about linking statically vs dynamically.) Of course the cat's out of the bag now, Valve probably can't go back to all the publishers and say "relink your binaries statically or your game won't work". So the other alternative is that Steam provides a full bizarro world's worth of libraries, which, like curro said, has to include libGL & co as well. And/or a cross-compiler onto their bizarro world (which would have the right binutils, gcc/libstdc++, etc). -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
On 11/03/15 16:01, Francisco Jerez wrote: Vivek Dasmohapatra writes: Hi - Hi, As you probably already know, there can only be one version of libstdc++.so in your runtime link chain That's a common misconception, in principle several versions of libstdc++.so with different DT_SONAME (i.e. with mutually incompatible ABIs) can be loaded at the same time, the GNU C++ runtime uses library and symbol versioning to achieve this. Of course two different versions with the same DT_SONAME cannot coexist, but that's not supposed to pose a problem because applications linked to the earlier version are expected to be forwards-compatible with more recent minor versions of the standard library. See [1]. This is usually not a problem, but when things are linked against the Steam runtime (for example), they can end up with two - one from the steam runtime, and one pulled in via the mesa dri libs from the OS/distribution. Last time I looked into this, it seemed to be a logical consequence of the inconsistency of the Steam runtime overriding system libraries: If they go as far as to override the operating system's copy of the C++ standard library and runtime with an outdated version (even though the OS libraries with the same DT_SONAME are expected to be backwards-compatible with Steam's), they should be shipping their own builds of libGL and DRI drivers as well. In order to address this, Valve asked Collabora to look at enabling mesa linking with libstdcc+.a/libgcc.a/libgcc_eh.a instead of listdcc++so and libgcc_s.so. I think I've figured out a minimally intrusive way to do this, working around the fact that libtool really, really wants to link those in if C++ is involved. I've broken this up into a couple of pieces: The first patch attached gets libtool to not link in the .so files in question: It's pretty small - it doesn't introduce a configure flag to control this behaviour, but I would be happy to adapt it to do so if that's considered a better approach. Honestly, I think that statically linking the C++ runtime is a hack, and it should be an opt-in based on some configure option if we want to support it at all. FWIW, here are NVIDIA OpenGL driver dependencies: $ ldd -r /usr/lib/nvidia-331-updates/libGL.so.1 /usr/lib/nvidia-331-updates/tls/libnvidia-tls.so.331.113 /usr/lib/nvidia-331-updates/libnvidia-glcore.so.331.113 /usr/lib/nvidia-331-updates/libGL.so.1: linux-vdso.so.1 => (0x7fff5cbfc000) libnvidia-tls.so.331.113 => /usr/lib/nvidia-331-updates/tls/libnvidia-tls.so.331.113 (0x7fa64be1f000) libnvidia-glcore.so.331.113 => /usr/lib/nvidia-331-updates/libnvidia-glcore.so.331.113 (0x7fa64961) libX11.so.6 => /usr/lib/x86_64-linux-gnu/libX11.so.6 (0x7fa6492d6000) libXext.so.6 => /usr/lib/x86_64-linux-gnu/libXext.so.6 (0x7fa6490c4000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7fa648cff000) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7fa648afa000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7fa6487f4000) libxcb.so.1 => /usr/lib/x86_64-linux-gnu/libxcb.so.1 (0x7fa6485d5000) /lib64/ld-linux-x86-64.so.2 (0x7fa64c37e000) libXau.so.6 => /usr/lib/x86_64-linux-gnu/libXau.so.6 (0x7fa6483d) libXdmcp.so.6 => /usr/lib/x86_64-linux-gnu/libXdmcp.so.6 (0x7fa6481ca000) /usr/lib/nvidia-331-updates/tls/libnvidia-tls.so.331.113: linux-vdso.so.1 => (0x7fff4d9fc000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f9a55f9) /lib64/ld-linux-x86-64.so.2 (0x7f9a56581000) /usr/lib/nvidia-331-updates/libnvidia-glcore.so.331.113: linux-vdso.so.1 => (0x7fff97f3c000) libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x7f1b8d064000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f1b8cc9f000) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x7f1b8ca9a000) /lib64/ld-linux-x86-64.so.2 (0x7f1b8fba2000) I can almost bet that NVIDIA uses C++ somewhere, but no dependencies on libstdc++. Not sure if they chose to statically link libstdc++ to support multiple distros, or to avoid issues like applications shipping their own libstdc++... Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Statically linking libstdc++ and libgcc
Hi - As you probably already know, there can only be one version of
libstdc++.so in your runtime link chain - This is usually not a problem,
but when things are linked against the Steam runtime (for example), they
can end up with two - one from the steam runtime, and one pulled in via
the mesa dri libs from the OS/distribution.
In order to address this, Valve asked Collabora to look at enabling
mesa linking with libstdcc+.a/libgcc.a/libgcc_eh.a instead of
listdcc++so and libgcc_s.so.
I think I've figured out a minimally intrusive way to do this, working
around the fact that libtool really, really wants to link those in
if C++ is involved.
I've broken this up into a couple of pieces:
The first patch attached gets libtool to not link in the .so
files in question: It's pretty small - it doesn't introduce a
configure flag to control this behaviour, but I would be happy
to adapt it to do so if that's considered a better approach.
The second and third extend this a little further: Patch #3
is actually to llvm, and builds a parallel libLLVM-X.Y-nostdlib.so
which is likewise linked with libstdc++.a et al instead of .so,
and patch #2 makes the mesa build system use said library if it
is available.
So: Would mesa be interested in patches #1 and/or #2?
If not, is there something I could do to make the patches
more palatable, or some other approach that you'd prefer?
( I'm aware of the configure flags to statically link against
libLLVM, but when I tried it with llvm-3.5 and a git checkout
of mesa 10.6-dev I got link errors, hence the LLVM patchset. )From 2c34ec2af17557ecf01bf8977af6ebfcc756e511 Mon Sep 17 00:00:00 2001
From: vivek
Date: Tue, 10 Mar 2015 19:41:53 +
Subject: [PATCH 1/2] Link libstdc++ and libgcc .a instead of .so
---
configure.ac| 20
src/gallium/Automake.inc| 1 +
src/gallium/targets/dri/Makefile.am | 5 +
3 files changed, 26 insertions(+)
diff --git a/configure.ac b/configure.ac
index 90c7737..3e509a6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -705,6 +705,25 @@ AC_ARG_ENABLE([dri],
[enable_dri="$enableval"],
[enable_dri=yes])
+dnl Strip out unnecessary dynamic linking in of libstdc++ and libgcc_s for
+dnl DRI modules: they cause problems when loaded by games linked against
+dnl a steam runtime with a different libgcc or libstdc++ version:
+if test x$enable_dri != xno;
+then
+AC_MSG_NOTICE([Cleanup libtool C++ postdeps: $postdeps_CXX (enable_dri=$enable_dri)])
+tmppdcxx=;
+for x in ${postdeps_CXX};
+do
+case $x in
+-lstdc++) true; ;;
+-lgcc_s) true; ;;
+*) tmppdcxx=${tmppdcxx}${tmppdcxx:+ }$x; ;;
+esac;
+done;
+postdeps_CXX="${tmppdcxx}";
+AC_MSG_NOTICE([Cleaned libtool C++ postdeps: $postdeps_CXX])
+fi
+
case "$host_os" in
linux*)
dri3_default=yes
@@ -2450,6 +2469,7 @@ echo "prefix: $prefix"
echo "exec_prefix: $exec_prefix"
echo "libdir: $libdir"
echo "includedir: $includedir"
+echo "postdeps_CXX:$postdeps_CXX"
dnl API info
echo ""
diff --git a/src/gallium/Automake.inc b/src/gallium/Automake.inc
index 95aae50..6de79c1 100644
--- a/src/gallium/Automake.inc
+++ b/src/gallium/Automake.inc
@@ -46,6 +46,7 @@ GALLIUM_TARGET_CFLAGS = \
GALLIUM_COMMON_LIB_DEPS = \
-lm \
+ -l:libgcc.a -l:libgcc_eh.a -l:libstdc++.a \
$(CLOCK_LIB) \
$(PTHREAD_LIBS) \
$(DLOPEN_LIBS)
diff --git a/src/gallium/targets/dri/Makefile.am b/src/gallium/targets/dri/Makefile.am
index aaeb950..5185273 100644
--- a/src/gallium/targets/dri/Makefile.am
+++ b/src/gallium/targets/dri/Makefile.am
@@ -27,6 +27,11 @@ gallium_dri_la_LDFLAGS = \
-shrext .so \
-module \
-avoid-version \
+ -static-libgcc \
+ -static-libstdc++ \
+ -l:libgcc.a \
+ -l:libstdc++.a \
+ -Wl,--exclude-libs -Wl,libgcc.a:libstdc++.a \
$(GC_SECTIONS)
if HAVE_LD_VERSION_SCRIPT
--
2.1.4
From 186c1ea9927035607822e3b19892b072044e145d Mon Sep 17 00:00:00 2001
From: vivek
Date: Tue, 10 Mar 2015 19:44:05 +
Subject: [PATCH 2/2] Use -nostdlib libLLVM-X.Y .so variant when present
---
configure.ac | 10 ++
1 file changed, 10 insertions(+)
diff --git a/configure.ac b/configure.ac
index 3e509a6..ac8f783 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2208,6 +2208,16 @@ if test "x$MESA_LLVM" != x0; then
if test "x$llvm_have_one_so" = xyes; then
dnl LLVM was built using auto*, so there is only one shared object.
LLVM_LIBS="-l$LLVM_SO_NAME"
+dnl Find a libLLVM-X.Y variant that doesn't need -lstdc++ -lgcc_s
+dnl Also: Make sure we don't ask for libstdc++ or libgcc_s explicitly:
+llvm_so_otherlibdeps=$($LLVM_CONFIG --ldflags)
+llvm_so_otherlibdeps=$(echo $llvm_so_otherlibdeps | sed -re 's@-lstdc\+\+|-lgcc_s@@g')
+AC_MSG_CHECKING([for a stdlib-independent lib$LLVM_SO_NAME])
+AC_CHECK_LIB([$LLVM_SO_N
[Mesa-dev] Ask for some HELP with Graphic Driver
Hello all I'm a college student. I have a project and have to write a graphic driver for a fixed function GPU that implemented on a FPGA. my question is : is that possible? and where and how can I start(I ask for some reference). I'm apologize for my poor english and if this is the wrong place to my question please notice me. best regards, Mostafa Farzane ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] gallium: implement get_device_vendor() for existing drivers
The only hackish ones are llvmpipe and softpipe, which currently return
the same string as for get_vendor(), while ideally they should return
the CPU vendor.
Signed-off-by: Giuseppe Bilotta
---
src/gallium/drivers/freedreno/freedreno_screen.c | 8
src/gallium/drivers/galahad/glhd_screen.c| 10 ++
src/gallium/drivers/i915/i915_screen.c | 7 +++
src/gallium/drivers/ilo/ilo_screen.c | 7 +++
src/gallium/drivers/llvmpipe/lp_screen.c | 1 +
src/gallium/drivers/noop/noop_pipe.c | 6 ++
src/gallium/drivers/nouveau/nouveau_screen.c | 7 +++
src/gallium/drivers/r300/r300_screen.c | 6 ++
src/gallium/drivers/radeon/r600_pipe_common.c| 6 ++
src/gallium/drivers/rbug/rbug_screen.c | 10 ++
src/gallium/drivers/softpipe/sp_screen.c | 1 +
src/gallium/drivers/svga/svga_screen.c | 1 +
src/gallium/drivers/trace/tr_screen.c| 22 ++
src/gallium/drivers/vc4/vc4_screen.c | 1 +
14 files changed, 93 insertions(+)
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index 31f596c..81d7f7a 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -96,6 +96,13 @@ fd_screen_get_vendor(struct pipe_screen *pscreen)
return "freedreno";
}
+static const char *
+fd_screen_get_device_vendor(struct pipe_screen *pscreen)
+{
+ return "Qualcomm";
+}
+
+
static uint64_t
fd_screen_get_timestamp(struct pipe_screen *pscreen)
{
@@ -531,6 +538,7 @@ fd_screen_create(struct fd_device *dev)
pscreen->get_name = fd_screen_get_name;
pscreen->get_vendor = fd_screen_get_vendor;
+ pscreen->get_device_vendor = fd_screen_get_device_vendor;
pscreen->get_timestamp = fd_screen_get_timestamp;
diff --git a/src/gallium/drivers/galahad/glhd_screen.c
b/src/gallium/drivers/galahad/glhd_screen.c
index 11ab1a9..9fafdbf 100644
--- a/src/gallium/drivers/galahad/glhd_screen.c
+++ b/src/gallium/drivers/galahad/glhd_screen.c
@@ -69,6 +69,15 @@ galahad_screen_get_vendor(struct pipe_screen *_screen)
return screen->get_vendor(screen);
}
+static const char *
+galahad_screen_get_device_vendor(struct pipe_screen *_screen)
+{
+ struct galahad_screen *glhd_screen = galahad_screen(_screen);
+ struct pipe_screen *screen = glhd_screen->screen;
+
+ return screen->get_device_vendor(screen);
+}
+
static int
galahad_screen_get_param(struct pipe_screen *_screen,
enum pipe_cap param)
@@ -361,6 +370,7 @@ galahad_screen_create(struct pipe_screen *screen)
GLHD_SCREEN_INIT(destroy);
GLHD_SCREEN_INIT(get_name);
GLHD_SCREEN_INIT(get_vendor);
+ GLHD_SCREEN_INIT(get_device_vendor);
GLHD_SCREEN_INIT(get_param);
GLHD_SCREEN_INIT(get_shader_param);
//GLHD_SCREEN_INIT(get_video_param);
diff --git a/src/gallium/drivers/i915/i915_screen.c
b/src/gallium/drivers/i915/i915_screen.c
index dc76464..9ab9c48 100644
--- a/src/gallium/drivers/i915/i915_screen.c
+++ b/src/gallium/drivers/i915/i915_screen.c
@@ -55,6 +55,12 @@ i915_get_vendor(struct pipe_screen *screen)
}
static const char *
+i915_get_device_vendor(struct pipe_screen *screen)
+{
+ return "Intel";
+}
+
+static const char *
i915_get_name(struct pipe_screen *screen)
{
static char buffer[128];
@@ -547,6 +553,7 @@ i915_screen_create(struct i915_winsys *iws)
is->base.get_name = i915_get_name;
is->base.get_vendor = i915_get_vendor;
+ is->base.get_device_vendor = i915_get_device_vendor;
is->base.get_param = i915_get_param;
is->base.get_shader_param = i915_get_shader_param;
is->base.get_paramf = i915_get_paramf;
diff --git a/src/gallium/drivers/ilo/ilo_screen.c
b/src/gallium/drivers/ilo/ilo_screen.c
index bf0a84a..80ea4c7 100644
--- a/src/gallium/drivers/ilo/ilo_screen.c
+++ b/src/gallium/drivers/ilo/ilo_screen.c
@@ -515,6 +515,12 @@ ilo_get_vendor(struct pipe_screen *screen)
}
static const char *
+ilo_get_device_vendor(struct pipe_screen *screen)
+{
+ return "Intel";
+}
+
+static const char *
ilo_get_name(struct pipe_screen *screen)
{
struct ilo_screen *is = ilo_screen(screen);
@@ -844,6 +850,7 @@ ilo_screen_create(struct intel_winsys *ws)
is->base.destroy = ilo_screen_destroy;
is->base.get_name = ilo_get_name;
is->base.get_vendor = ilo_get_vendor;
+ is->base.get_device_vendor = ilo_get_device_vendor;
is->base.get_param = ilo_get_param;
is->base.get_paramf = ilo_get_paramf;
is->base.get_shader_param = ilo_get_shader_param;
diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c
b/src/gallium/drivers/llvmpipe/lp_screen.c
index 3387d3a..4b45725 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -589,6 +589,7 @@ llvmpipe_create_screen(struct sw_winsys *winsys)
screen->base.get_name = l
[Mesa-dev] [PATCH 2/4] gallium: introduce get_device_vendor() entrypoint for pipes
This will be needed by Clover to return the correct information
to CL_DEVICE_VENDOR info queries.
Signed-off-by: Giuseppe Bilotta
Reviewed-by: Michel Dänzer
---
src/gallium/docs/source/screen.rst | 6 ++
src/gallium/include/pipe/p_screen.h | 8
2 files changed, 14 insertions(+)
diff --git a/src/gallium/docs/source/screen.rst
b/src/gallium/docs/source/screen.rst
index e0fd1a2..f576c7f 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -460,6 +460,12 @@ get_vendor
Returns the screen vendor.
+get_device_vendor
+^
+
+Returns the actual vendor of the device driving the screen
+(as opposed to the driver vendor).
+
.. _get_param:
get_param
diff --git a/src/gallium/include/pipe/p_screen.h
b/src/gallium/include/pipe/p_screen.h
index 4018f8a..21e7dd3 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -72,6 +72,14 @@ struct pipe_screen {
const char *(*get_vendor)( struct pipe_screen * );
/**
+* Returns the device vendor.
+*
+* The returned value should return the actual device vendor/manufacturer,
+* rather than a potentially generic driver string.
+*/
+ const char *(*get_device_vendor)( struct pipe_screen * );
+
+ /**
* Query an integer-valued capability/parameter/limit
* \param param one of PIPE_CAP_x
*/
--
2.1.2.766.gaa23a90
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] xlib: use strdup() instead of _mesa_strdup()
--- src/mesa/drivers/x11/fakeglx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c index 3869e94..4fd6d75 100644 --- a/src/mesa/drivers/x11/fakeglx.c +++ b/src/mesa/drivers/x11/fakeglx.c @@ -40,6 +40,7 @@ */ +#include #include #include "glxheader.h" #include "glxapi.h" @@ -846,7 +847,7 @@ register_with_display(Display *dpy) ext = dpy->ext_procs; /* new extension is at head of list */ assert(c->extension == ext->codes.extension); (void) c; /* silence warning */ - ext->name = _mesa_strdup(extName); + ext->name = strdup(extName); ext->close_display = close_display_callback; } } -- 1.9.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: use strdup() instead of _mesa_strdup()
On 03/10/2015 08:47 PM, Ian Romanick wrote: On 03/10/2015 07:47 PM, Ian Romanick wrote: On 03/10/2015 06:42 PM, Brian Paul wrote: We were already using strdup() in various places in Mesa. Get rid of the _mesa_strdup() wrapper. All the callers pass a non-NULL argument so the NULL check isn't needed either. --- src/mesa/main/imports.c | 18 -- src/mesa/main/imports.h | 3 --- src/mesa/main/objectlabel.c | 2 +- src/mesa/main/shaderapi.c | 2 +- src/mesa/main/transformfeedback.c | 2 +- src/mesa/program/prog_instruction.c | 2 +- src/mesa/program/prog_parameter.c | 2 +- src/mesa/program/prog_statevars.c | 2 +- src/mesa/program/program.c | 6 +++--- A quick 'grep -lr _mesa_strdup src/ | sort' shows the following files: src/gallium/state_trackers/glx/xlib/glx_api.c src/mesa/drivers/x11/fakeglx.c src/mesa/main/imports.c src/mesa/main/imports.h src/mesa/main/objectlabel.c src/mesa/main/shaderapi.c src/mesa/main/transformfeedback.c src/mesa/program/prog_instruction.c src/mesa/program/prog_parameter.c src/mesa/program/program.c src/mesa/program/prog_statevars.c It looks like you missed a couple: src/gallium/state_trackers/glx/xlib/glx_api.c src/mesa/drivers/x11/fakeglx.c But I guess patch 1 got the first one. I missed a patch when I mailed this series to the list. I'll post it shortly. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89553] Account request
https://bugs.freedesktop.org/show_bug.cgi?id=89553 Bug ID: 89553 Summary: Account request Product: Mesa Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: el...@igalia.com QA Contact: mesa-dev@lists.freedesktop.org CC: ito...@igalia.com, ja...@jlekstrand.net, sigles...@igalia.com Created attachment 114243 --> https://bugs.freedesktop.org/attachment.cgi?id=114243&action=edit SSH public key I have been contributing to Mesa and Piglit projects for the last months, and I plan to continue doing so. I have authored ~20 commits that have been integrated upstream. Please, consider granting me commit rights to Mesa and Piglit git repositories. Real name: Eduardo Lima Mitev Email address: el...@igalia.com Preferred account name: elima GPG and SSH keys follow. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89553] Account request
https://bugs.freedesktop.org/show_bug.cgi?id=89553 --- Comment #1 from Eduardo Lima Mitev --- Created attachment 114244 --> https://bugs.freedesktop.org/attachment.cgi?id=114244&action=edit PGP public key -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] i965/state: Allow brw->atoms[] to be used by multiple pipelines
Convert brw->num_atoms to an int array.
Adds brw_add_pipeline_atoms which copies the atoms for a pipeline, and
sets brw->num_atoms[p] for pipeline p.
Signed-off-by: Jordan Justen
---
src/mesa/drivers/dri/i965/brw_context.h | 8 +++-
src/mesa/drivers/dri/i965/brw_state_upload.c | 72 +---
2 files changed, 50 insertions(+), 30 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_context.h
b/src/mesa/drivers/dri/i965/brw_context.h
index 682fbe9..fd43b07 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -149,6 +149,12 @@ struct brw_vue_prog_key;
struct brw_wm_prog_key;
struct brw_wm_prog_data;
+enum brw_pipeline {
+ BRW_RENDER_PIPELINE,
+
+ BRW_NUM_PIPELINES
+};
+
enum brw_cache_id {
BRW_CACHE_FS_PROG,
BRW_CACHE_BLORP_BLIT_PROG,
@@ -1386,7 +1392,7 @@ struct brw_context
int entries_per_oa_snapshot;
} perfmon;
- int num_atoms;
+ int num_atoms[BRW_NUM_PIPELINES];
const struct brw_tracked_state atoms[57];
/* If (INTEL_DEBUG & DEBUG_BATCH) */
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 3b64a05..dbfdc92 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -342,11 +342,42 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
}
}
+static inline const struct brw_tracked_state *
+brw_pipeline_first_atom(struct brw_context *brw,
+enum brw_pipeline pipeline)
+{
+ const struct brw_tracked_state *atom = &brw->atoms[0];
+ for (int i = 0; i < pipeline; i++)
+ atom += brw->num_atoms[i];
+ return atom;
+}
+
+static void
+brw_add_pipeline_atoms(struct brw_context *brw,
+ enum brw_pipeline pipeline,
+ const struct brw_tracked_state **atoms,
+ int num_atoms)
+{
+ /* This is to work around brw_context::atoms being declared const. We want
+* it to be const, but it needs to be initialized somehow!
+*/
+ struct brw_tracked_state *context_atoms =
+ (struct brw_tracked_state *) brw_pipeline_first_atom(brw, pipeline);
+
+ assert(context_atoms + num_atoms <= brw->atoms + ARRAY_SIZE(brw->atoms));
+
+ for (int i = 0; i < num_atoms; i++) {
+ context_atoms[i] = *atoms[i];
+ assert(context_atoms[i].dirty.mesa | context_atoms[i].dirty.brw);
+ assert(context_atoms[i].emit);
+ }
+
+ brw->num_atoms[pipeline] = num_atoms;
+}
+
void brw_init_state( struct brw_context *brw )
{
struct gl_context *ctx = &brw->ctx;
- const struct brw_tracked_state **atoms;
- int num_atoms;
STATIC_ASSERT(ARRAY_SIZE(gen4_atoms) <= ARRAY_SIZE(brw->atoms));
STATIC_ASSERT(ARRAY_SIZE(gen6_atoms) <= ARRAY_SIZE(brw->atoms));
@@ -356,34 +387,17 @@ void brw_init_state( struct brw_context *brw )
brw_init_caches(brw);
if (brw->gen >= 8) {
- atoms = gen8_atoms;
- num_atoms = ARRAY_SIZE(gen8_atoms);
+ brw_add_pipeline_atoms(brw, BRW_RENDER_PIPELINE,
+ gen8_atoms, ARRAY_SIZE(gen8_atoms));
} else if (brw->gen == 7) {
- atoms = gen7_atoms;
- num_atoms = ARRAY_SIZE(gen7_atoms);
+ brw_add_pipeline_atoms(brw, BRW_RENDER_PIPELINE,
+ gen7_atoms, ARRAY_SIZE(gen7_atoms));
} else if (brw->gen == 6) {
- atoms = gen6_atoms;
- num_atoms = ARRAY_SIZE(gen6_atoms);
+ brw_add_pipeline_atoms(brw, BRW_RENDER_PIPELINE,
+ gen6_atoms, ARRAY_SIZE(gen6_atoms));
} else {
- atoms = gen4_atoms;
- num_atoms = ARRAY_SIZE(gen4_atoms);
- }
-
- brw->num_atoms = num_atoms;
-
- /* This is to work around brw_context::atoms being declared const. We want
-* it to be const, but it needs to be initialized somehow!
-*/
- struct brw_tracked_state *context_atoms =
- (struct brw_tracked_state *) &brw->atoms[0];
-
- for (int i = 0; i < num_atoms; i++)
- context_atoms[i] = *atoms[i];
-
- while (num_atoms--) {
- assert((*atoms)->dirty.mesa | (*atoms)->dirty.brw);
- assert((*atoms)->emit);
- atoms++;
+ brw_add_pipeline_atoms(brw, BRW_RENDER_PIPELINE,
+ gen4_atoms, ARRAY_SIZE(gen4_atoms));
}
brw_upload_initial_gpu_state(brw);
@@ -631,7 +645,7 @@ void brw_upload_render_state(struct brw_context *brw)
memset(&examined, 0, sizeof(examined));
prev = *state;
- for (i = 0; i < brw->num_atoms; i++) {
+ for (i = 0; i < brw->num_atoms[BRW_RENDER_PIPELINE]; i++) {
const struct brw_tracked_state *atom = &brw->atoms[i];
struct brw_state_flags generated;
@@ -651,7 +665,7 @@ void brw_upload_render_state(struct brw_context *brw)
}
}
else {
- for (i = 0; i < brw->num_atoms; i++) {
+ for (i = 0; i < brw->num_atoms[BRW_RENDER_PIPELINE]; i++) {
const struct brw_tracked_state *atom = &brw->atoms[i];
[Mesa-dev] [PATCH 0/4] Support multiple state pipelines for i965
git://people.freedesktop.org/~jljusten/mesa i965-pipelines Gen8 appears to require a separate pipeline for CS support. Here is another take on the multiple pipelines idea. In contrast to the previous multiple pipeline series, it preserves usage of brw->state.dirty in most places. Now, it saves off the dirty state for the other pipelines at state upload time. This seems to be a lot less intrusive than saving the dirty state for every pipeline whenever new dirty states are flagged. I tested this for render via jenkins, and for CS on gen7. Jordan Justen (4): i965/state: Rename brw_upload_state to brw_upload_render_state i965/state: Allow brw->atoms[] to be used by multiple pipelines i965/state: Create separate dirty state bits for each pipeline i965/state: Add compute pipeline with empty atom lists src/mesa/drivers/dri/i965/brw_context.h | 10 +- src/mesa/drivers/dri/i965/brw_draw.c | 2 +- src/mesa/drivers/dri/i965/brw_state.h| 3 +- src/mesa/drivers/dri/i965/brw_state_upload.c | 137 +++ 4 files changed, 112 insertions(+), 40 deletions(-) -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
Vivek Dasmohapatra writes: > Hi - Hi, > As you probably already know, there can only be one version of > libstdc++.so in your runtime link chain That's a common misconception, in principle several versions of libstdc++.so with different DT_SONAME (i.e. with mutually incompatible ABIs) can be loaded at the same time, the GNU C++ runtime uses library and symbol versioning to achieve this. Of course two different versions with the same DT_SONAME cannot coexist, but that's not supposed to pose a problem because applications linked to the earlier version are expected to be forwards-compatible with more recent minor versions of the standard library. See [1]. > This is usually not a problem, but when things are linked against the > Steam runtime (for example), they can end up with two - one from the > steam runtime, and one pulled in via the mesa dri libs from the > OS/distribution. > Last time I looked into this, it seemed to be a logical consequence of the inconsistency of the Steam runtime overriding system libraries: If they go as far as to override the operating system's copy of the C++ standard library and runtime with an outdated version (even though the OS libraries with the same DT_SONAME are expected to be backwards-compatible with Steam's), they should be shipping their own builds of libGL and DRI drivers as well. > In order to address this, Valve asked Collabora to look at enabling > mesa linking with libstdcc+.a/libgcc.a/libgcc_eh.a instead of > listdcc++so and libgcc_s.so. > > I think I've figured out a minimally intrusive way to do this, working > around the fact that libtool really, really wants to link those in > if C++ is involved. > > I've broken this up into a couple of pieces: > > The first patch attached gets libtool to not link in the .so > files in question: It's pretty small - it doesn't introduce a > configure flag to control this behaviour, but I would be happy > to adapt it to do so if that's considered a better approach. > Honestly, I think that statically linking the C++ runtime is a hack, and it should be an opt-in based on some configure option if we want to support it at all. > The second and third extend this a little further: Patch #3 > is actually to llvm, and builds a parallel libLLVM-X.Y-nostdlib.so > which is likewise linked with libstdc++.a et al instead of .so, > and patch #2 makes the mesa build system use said library if it > is available. > > So: Would mesa be interested in patches #1 and/or #2? > If not, is there something I could do to make the patches > more palatable, or some other approach that you'd prefer? > > ( I'm aware of the configure flags to statically link against >libLLVM, but when I tried it with llvm-3.5 and a git checkout >of mesa 10.6-dev I got link errors, hence the LLVM patchset. ) [1] https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html >[...] signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/9] radeonsi: add support for easy opcodes from ARB_gpu_shader5
It seems overkill to use LLVM IR for BFE. There are several ways to express BFE(value, offset, bits) exactly, for example: # if bits == 0: # 0 # else if offset + bits < 32: # (value << (32 - offset - bits)) >> (32 - bits) # else: # value >> offset Which can be simplified to either: # both = offset + bits # if bits == 0: # 0 # else if both < 32: # (value << (32 - both)) >> (32 - bits) # else: # value >> offset Or: # if bits == 0: # 0 # else if offset + bits < 32: # inv_bits = 32 - bits # (value << (inv_bits - offset)) >> inv_bits # else: # value >> offset All 3 variants should use ASHR if the signed version is required. Alternatively, one can use this as well: # if bits == 32: # value # else: # (value >> offset) & ((1 << bits) - 1) And an explicit sign extension should be used for the signed version. The first three are so complex that I doubt it would be easy to recognize them and match them precisely. They are also very unlikely to appear open-coded in real applications, therefore, expanding them and then matching them again seems like a huge waste of time. Apps will likely just use LSHR+AND, which extracts a bitfield, but it's not the same as BFE, so BFE can't be used for that. (if the second operand of AND is a constant expression, then BFE can indeed be used) I agree that we could use LLVM IR for simple instructions like BFM and BFI, but I doubt it's a good idea for complex instructions like BFE, and any small optimization that changes the expression can break the matching. Marek On Tue, Mar 10, 2015 at 3:30 PM, Tom Stellard wrote: > On Tue, Mar 10, 2015 at 12:42:38PM +0100, Marek Olšák wrote: >> OK. What about patches 8 an 9? >> > > I think the intrinsics in 9 are OK, but 8 should be using LLVM IR. > > -Tom > >> Marek >> >> On Thu, Mar 5, 2015 at 8:30 PM, Tom Stellard wrote: >> > On Mon, Mar 02, 2015 at 12:54:21PM +0100, Marek Olšák wrote: >> >> From: Marek Olšák >> >> >> > >> > Hi Marek, >> > >> > After discussing with Matt, I think we should use LLVM IR rather than >> > intrinsics for IBFE and UBFE and then add patterns for them either in >> > the TableGen Files or AMDGPUISelDAGToDAG.cpp. >> > >> > Using intrinsics for BREV and POPC is fine though. >> > >> > -Tom >> > >> >> --- >> >> src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 8 >> >> 1 file changed, 8 insertions(+) >> >> >> >> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c >> >> b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c >> >> index 385d3ad..034095f 100644 >> >> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c >> >> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c >> >> @@ -1293,6 +1293,8 @@ void radeon_llvm_context_init(struct >> >> radeon_llvm_context * ctx) >> >> bld_base->op_actions[TGSI_OPCODE_AND].emit = emit_and; >> >> bld_base->op_actions[TGSI_OPCODE_ARL].emit = emit_arl; >> >> bld_base->op_actions[TGSI_OPCODE_BGNLOOP].emit = bgnloop_emit; >> >> + bld_base->op_actions[TGSI_OPCODE_BREV].emit = >> >> build_tgsi_intrinsic_nomem; >> >> + bld_base->op_actions[TGSI_OPCODE_BREV].intr_name = >> >> "llvm.AMDGPU.brev"; >> >> bld_base->op_actions[TGSI_OPCODE_BRK].emit = brk_emit; >> >> bld_base->op_actions[TGSI_OPCODE_CEIL].emit = >> >> build_tgsi_intrinsic_nomem; >> >> bld_base->op_actions[TGSI_OPCODE_CEIL].intr_name = "ceil"; >> >> @@ -1326,6 +1328,8 @@ void radeon_llvm_context_init(struct >> >> radeon_llvm_context * ctx) >> >> bld_base->op_actions[TGSI_OPCODE_FSNE].emit = emit_fcmp; >> >> bld_base->op_actions[TGSI_OPCODE_IABS].emit = >> >> build_tgsi_intrinsic_nomem; >> >> bld_base->op_actions[TGSI_OPCODE_IABS].intr_name = >> >> "llvm.AMDIL.abs."; >> >> + bld_base->op_actions[TGSI_OPCODE_IBFE].emit = >> >> build_tgsi_intrinsic_nomem; >> >> + bld_base->op_actions[TGSI_OPCODE_IBFE].intr_name = >> >> "llvm.AMDGPU.bfe.i32"; >> >> bld_base->op_actions[TGSI_OPCODE_IDIV].emit = emit_idiv; >> >> bld_base->op_actions[TGSI_OPCODE_IF].emit = if_emit; >> >> bld_base->op_actions[TGSI_OPCODE_UIF].emit = uif_emit; >> >> @@ -1350,6 +1354,8 @@ void radeon_llvm_context_init(struct >> >> radeon_llvm_context * ctx) >> >> bld_base->op_actions[TGSI_OPCODE_MOD].emit = emit_mod; >> >> bld_base->op_actions[TGSI_OPCODE_NOT].emit = emit_not; >> >> bld_base->op_actions[TGSI_OPCODE_OR].emit = emit_or; >> >> + bld_base->op_actions[TGSI_OPCODE_POPC].emit = >> >> build_tgsi_intrinsic_nomem; >> >> + bld_base->op_actions[TGSI_OPCODE_POPC].intr_name = "llvm.ctpop.i32"; >> >> bld_base->op_actions[TGSI_OPCODE_POW].emit = >> >> build_tgsi_intrinsic_nomem; >> >> bld_base->op_actions[TGSI_OPCODE_POW].intr_name = "llvm.pow.f32"; >> >> bld_base->op_actions[TGSI_OPCODE_ROUND].emit = >> >> build_tgsi_intrinsic_nomem; >> >> @@ -1389,6 +1395,8 @@ void radeon_llvm_context_init(struct >> >> radeon_llvm_context * ctx) >> >
[Mesa-dev] [Bug 35268] initial-exec TLS model breaks dlopen'ed libGL
https://bugs.freedesktop.org/show_bug.cgi?id=35268 Timo Teräs changed: What|Removed |Added CC||timo.te...@iki.fi -- You are receiving this mail because: You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH ] vbo: improve the code style by adjust the preprocessing c code directives.
On 03/09/2015 01:44 PM, marius.pre...@intel.com wrote:
From: Marius Predut
Brain Paul review suggestion: there's more macro use here than necessary.
"Brian"
Removed and redefine some #define preprocessing directives.
Removed the directive input parameter 'T' .
No functional changes.
Signed-off-by: Marius Predut
Looks good. Thanks. I presume you did a piglit test to verify the change.
Reviewed-by: Brian Paul
---
src/mesa/vbo/vbo_attrib_tmp.h | 74 ++---
src/mesa/vbo/vbo_exec_api.c |2 +-
2 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
index 80e8aaf..348dd77 100644
--- a/src/mesa/vbo/vbo_attrib_tmp.h
+++ b/src/mesa/vbo/vbo_attrib_tmp.h
@@ -30,35 +30,30 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
/* ATTR */
-#define ATTR( A, N, T, V0, V1, V2, V3 ) \
-ATTR_##T((A), (N), (T), (V0), (V1), (V2), (V3))
-
-#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
-ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), \
-UINT_AS_UNION(V2), UINT_AS_UNION(V3))
-#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
-ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), \
+#define ATTRI( A, N, V0, V1, V2, V3 ) \
+ATTR_UNION(A, N, GL_INT, INT_AS_UNION(V0), INT_AS_UNION(V1), \
INT_AS_UNION(V2), INT_AS_UNION(V3))
-#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 ) \
-ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1),\
+#define ATTRUI( A, N, V0, V1, V2, V3 ) \
+ATTR_UNION(A, N, GL_UNSIGNED_INT, UINT_AS_UNION(V0), UINT_AS_UNION(V1), \
+UINT_AS_UNION(V2), UINT_AS_UNION(V3))
+#define ATTRF( A, N, V0, V1, V2, V3 ) \
+ATTR_UNION(A, N, GL_FLOAT, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1),\
FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
/* float */
-#define ATTR1FV( A, V ) ATTR( A, 1, GL_FLOAT, (V)[0], 0, 0, 1 )
-#define ATTR2FV( A, V ) ATTR( A, 2, GL_FLOAT, (V)[0], (V)[1], 0, 1 )
-#define ATTR3FV( A, V ) ATTR( A, 3, GL_FLOAT, (V)[0], (V)[1], (V)[2], 1 )
-#define ATTR4FV( A, V ) ATTR( A, 4, GL_FLOAT, (V)[0], (V)[1], (V)[2], (V)[3] )
+#define ATTR1FV( A, V ) ATTRF( A, 1, (V)[0], 0, 0, 1 )
+#define ATTR2FV( A, V ) ATTRF( A, 2, (V)[0], (V)[1], 0, 1 )
+#define ATTR3FV( A, V ) ATTRF( A, 3, (V)[0], (V)[1], (V)[2], 1 )
+#define ATTR4FV( A, V ) ATTRF( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
-#define ATTR1F( A, X ) ATTR( A, 1, GL_FLOAT, X, 0, 0, 1 )
-#define ATTR2F( A, X, Y ) ATTR( A, 2, GL_FLOAT, X, Y, 0, 1 )
-#define ATTR3F( A, X, Y, Z )ATTR( A, 3, GL_FLOAT, X, Y, Z, 1 )
-#define ATTR4F( A, X, Y, Z, W ) ATTR( A, 4, GL_FLOAT, X, Y, Z, W )
+#define ATTR1F( A, X ) ATTRF( A, 1, X, 0, 0, 1 )
+#define ATTR2F( A, X, Y ) ATTRF( A, 2, X, Y, 0, 1 )
+#define ATTR3F( A, X, Y, Z )ATTRF( A, 3, X, Y, Z, 1 )
+#define ATTR4F( A, X, Y, Z, W ) ATTRF( A, 4, X, Y, Z, W )
-/* int */
-#define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
- X, Y, Z, W )
+/* int */
#define ATTR2IV( A, V ) ATTRI( A, 2, (V)[0], (V)[1], 0, 1 )
#define ATTR3IV( A, V ) ATTRI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
#define ATTR4IV( A, V ) ATTRI( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
@@ -70,9 +65,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
/* uint */
-#define ATTRUI( A, N, X, Y, Z, W) ATTR( A, N, GL_UNSIGNED_INT, \
-X, Y, Z, W )
-
#define ATTR2UIV( A, V ) ATTRUI( A, 2, (V)[0], (V)[1], 0, 1 )
#define ATTR3UIV( A, V ) ATTRUI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
#define ATTR4UIV( A, V ) ATTRUI( A, 4, (V)[0], (V)[1], (V)[2], (V)[3] )
@@ -82,7 +74,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
#define ATTR3UI( A, X, Y, Z )ATTRUI( A, 3, X, Y, Z, 1 )
#define ATTR4UI( A, X, Y, Z, W ) ATTRUI( A, 4, X, Y, Z, W )
-#define MAT_ATTR( A, N, V ) ATTR( A, N, GL_FLOAT, (V)[0], (V)[1], (V)[2],
(V)[3] )
+#define MAT_ATTR( A, N, V ) ATTRF( A, N, (V)[0], (V)[1], (V)[2], (V)[3] )
static inline float conv_ui10_to_norm_float(unsigned ui10)
{
@@ -94,20 +86,20 @@ static inline float conv_ui2_to_norm_float(unsigned ui2)
return ui2 / 3.0f;
}
-#define ATTRUI10_1( A, UI ) ATTR( A, 1, GL_FLOAT, (UI) & 0x3ff, 0, 0, 1 )
-#define ATTRUI10_2( A, UI ) ATTR( A, 2, GL_FLOAT, (UI) & 0x3ff, ((UI) >> 10) &
0x3ff, 0, 1 )
-#define ATTRUI10_3( A, UI ) ATTR( A, 3, GL_FLOAT, (UI) & 0x3ff, ((UI) >> 10) & 0x3ff,
((UI) >> 20) & 0x3ff, 1 )
-#define ATTRUI10_4( A, UI ) ATTR( A, 4, GL_FLOAT, (UI) & 0x3ff, ((UI) >> 10) & 0x3ff, ((UI)
>> 20) & 0x3ff, ((UI) >> 30) & 0x3 )
+#define ATTRUI10_1( A, UI ) ATTRF( A, 1, (UI) & 0x3ff, 0, 0, 1 )
+#define ATTRUI10_2( A, UI ) ATTRF( A, 2, (UI) & 0x3ff, ((UI) >> 10) & 0x3ff,
0, 1 )
+#define ATTRUI10_3( A, UI ) ATTRF( A, 3, (UI) & 0x3ff, ((UI) >> 10) & 0x3ff, ((UI)
>> 20) & 0x3ff, 1 )
+#define ATTRUI10_4( A, UI ) ATTRF( A, 4, (UI) & 0x3ff, ((UI) >> 10) & 0x3ff, ((UI) >> 20)
& 0x3ff, ((UI) >> 30) & 0x3 )
-#define ATTRUI10N_1( A, UI ) ATT
[Mesa-dev] [PATCH 4/4 v3] mesa: Check for valid PBO access in gl(Compressed)Tex(Sub)Image calls
This patch adds two types of checks to the gl(Compressed)Tex(Sub)Imgage family
of functions when a pixel buffer object is bound to GL_PIXEL_UNPACK_BUFFER:
- That the buffer is not mapped.
- The total data size is within the boundaries of the buffer size.
It does so by calling auxiliary validations functions from PBO API:
_mesa_validate_pbo_source() for non-compressed texture calls, and
_mesa_validate_pbo_source_compressed() for compressed texture calls.
The first check is defined in Section 6.3.2 'Effects of Mapping Buffers
on Other GL Commands' of the GLES 3.1 spec, page 57:
"Any GL command which attempts to read from, write to, or change the
state of a buffer object may generate an INVALID_OPERATION error if all
or part of the buffer object is mapped. However, only commands which
explicitly describe this error are required to do so. If an error is not
generated, using such commands to perform invalid reads, writes, or
state changes will have undefined results and may result in GL
interruption or termination."
Similar wording exists in GL 4.5 spec, page 76.
In the case of gl(Compressed)Tex(Sub)Image(2,3)D, the specification doesn't
force
implemtations to throw an error. However since Mesa don't currently implement
checks to determine when it is safe to read/write from/to a mapped PBO, we
should always return the error if all or parts of it are mapped.
The 2nd check is defined in Section 8.5 'Texture Image Specification' of the
OpenGL 4.5 spec, page 203:
"An INVALID_OPERATION error is generated if a pixel unpack buffer object
is bound and storing texture data would access memory beyond the end of
the pixel unpack buffer."
Fixes 4 dEQP tests:
*
dEQP-GLES3.functional.negative_api.texture.compressedteximage2d_invalid_buffer_target
*
dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_buffer_target
*
dEQP-GLES3.functional.negative_api.texture.compressedteximage3d_invalid_buffer_target
*
dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target
---
src/mesa/main/teximage.c | 180 +++
1 file changed, 103 insertions(+), 77 deletions(-)
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 7b1a0e6..64e4816 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -53,6 +53,7 @@
#include "mtypes.h"
#include "glformats.h"
#include "texstore.h"
+#include "pbo.h"
/**
@@ -1619,32 +1620,30 @@ error_check_subtexture_dimensions(struct gl_context
*ctx, GLuint dims,
/* Check size */
if (subWidth < 0) {
_mesa_error(ctx, GL_INVALID_VALUE,
- "%s%dD(width=%d)", func, dims, subWidth);
+ "%s(width=%d)", func, subWidth);
return GL_TRUE;
}
if (dims > 1 && subHeight < 0) {
_mesa_error(ctx, GL_INVALID_VALUE,
- "%s%dD(height=%d)", func, dims, subHeight);
+ "%s(height=%d)", func, subHeight);
return GL_TRUE;
}
if (dims > 2 && subDepth < 0) {
_mesa_error(ctx, GL_INVALID_VALUE,
- "%s%dD(depth=%d)", func, dims, subDepth);
+ "%s(depth=%d)", func, subDepth);
return GL_TRUE;
}
/* check xoffset and width */
if (xoffset < - (GLint) destImage->Border) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s%dD(xoffset)",
- func, dims);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(xoffset)", func);
return GL_TRUE;
}
if (xoffset + subWidth > (GLint) destImage->Width) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s%dD(xoffset+width)",
- func, dims);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(xoffset+width)", func);
return GL_TRUE;
}
@@ -1652,13 +1651,11 @@ error_check_subtexture_dimensions(struct gl_context
*ctx, GLuint dims,
if (dims > 1) {
GLint yBorder = (target == GL_TEXTURE_1D_ARRAY) ? 0 : destImage->Border;
if (yoffset < -yBorder) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s%dD(yoffset)",
- func, dims);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(yoffset)", func);
return GL_TRUE;
}
if (yoffset + subHeight > (GLint) destImage->Height) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s%dD(yoffset+height)",
- func, dims);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(yoffset+height)", func);
return GL_TRUE;
}
}
@@ -1671,7 +1668,7 @@ error_check_subtexture_dimensions(struct gl_context *ctx,
GLuint dims,
0 : destImage->Border;
if (zoffset < -zBorder) {
- _mesa_error(ctx, GL_INVALID_VALUE, "%s3D(zoffset)", func);
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(zoffset)", func);
return GL_TRUE;
}
@@ -1679,7 +1676,7 @@ error_check_subtexture_dimensions(struct gl_context *ctx,
GLuint dims,
if (target == GL_TEXTURE_CUBE_MAP)
Re: [Mesa-dev] Statically linking libstdc++ and libgcc
On 11.03.2015 05:07, Vivek Dasmohapatra wrote: > Hi - As you probably already know, there can only be one version of > libstdc++.so in your runtime link chain - This is usually not a problem, > but when things are linked against the Steam runtime (for example), they > can end up with two - one from the steam runtime, and one pulled in via > the mesa dri libs from the OS/distribution. How can that happen? The problems I've seen related to this were usually because Steam overrides the system libstdc++ / libgcc with older versions, which breaks other system libraries. Can somebody please tell Valve that doing that is not okay? And it's not necessary either. Each library can be compared to the system version individually and only overridden when necessary, e.g. by putting each library into its own directory and only adding the necessary directories to $LD_LIBRARY_PATH. E.g. VMware use this approach in their products. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4 v3] mesa: Separate PBO validation checks from buffer mapping, to allow reuse
Internal PBO functions such as _mesa_map_validate_pbo_source() and
_mesa_validate_pbo_compressed_teximage() perform validation and buffer mapping
within the same call.
This patch takes out the validation into separate functions to allow reuse
of functionality by other code (i.e, gl(Compressed)Tex(Sub)Image).
---
src/mesa/main/pbo.c | 117 ++--
src/mesa/main/pbo.h | 14 +++
2 files changed, 100 insertions(+), 31 deletions(-)
diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
index 259f763..85c32a1 100644
--- a/src/mesa/main/pbo.c
+++ b/src/mesa/main/pbo.c
@@ -164,23 +164,18 @@ _mesa_map_pbo_source(struct gl_context *ctx,
return buf;
}
-
/**
- * Combine PBO-read validation and mapping.
- * If any GL errors are detected, they'll be recorded and NULL returned.
+ * Perform PBO validation for read operations with uncompressed textures.
+ * If any GL errors are detected, false is returned, otherwise returns true.
* \sa _mesa_validate_pbo_access
- * \sa _mesa_map_pbo_source
- * A call to this function should have a matching call to
- * _mesa_unmap_pbo_source().
*/
-const GLvoid *
-_mesa_map_validate_pbo_source(struct gl_context *ctx,
- GLuint dimensions,
- const struct gl_pixelstore_attrib *unpack,
- GLsizei width, GLsizei height, GLsizei depth,
- GLenum format, GLenum type,
- GLsizei clientMemSize,
- const GLvoid *ptr, const char *where)
+bool
+_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,
+ const struct gl_pixelstore_attrib *unpack,
+ GLsizei width, GLsizei height, GLsizei depth,
+ GLenum format, GLenum type,
+ GLsizei clientMemSize,
+ const GLvoid *ptr, const char *where)
{
assert(dimensions == 1 || dimensions == 2 || dimensions == 3);
@@ -188,24 +183,85 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,
format, type, clientMemSize, ptr)) {
if (_mesa_is_bufferobj(unpack->BufferObj)) {
_mesa_error(ctx, GL_INVALID_OPERATION,
- "%s(out of bounds PBO access)", where);
+ "%s(out of bounds access)",
+ where);
} else {
_mesa_error(ctx, GL_INVALID_OPERATION,
"%s(out of bounds access: bufSize (%d) is too small)",
where, clientMemSize);
}
- return NULL;
+ return false;
}
if (!_mesa_is_bufferobj(unpack->BufferObj)) {
/* non-PBO access: no further validation to be done */
- return ptr;
+ return true;
}
if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
/* buffer is already mapped - that's an error */
- _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", where);
- return NULL;
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
+ where);
+ return false;
+ }
+
+ return true;
+}
+
+/**
+ * Perform PBO validation for read operations with compressed textures.
+ * If any GL errors are detected, false is returned, otherwise returns true.
+ */
+bool
+_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint dimensions,
+ const struct gl_pixelstore_attrib *unpack,
+ GLsizei imageSize, const GLvoid *pixels,
+ const char *where)
+{
+ if (!_mesa_is_bufferobj(unpack->BufferObj)) {
+ /* not using a PBO */
+ return true;
+ }
+
+ if ((const GLubyte *) pixels + imageSize >
+ ((const GLubyte *) 0) + unpack->BufferObj->Size) {
+ /* out of bounds read! */
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid PBO access)",
+ where);
+ return false;
+ }
+
+ if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
+ /* buffer is already mapped - that's an error */
+ _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
+ where);
+ return false;
+ }
+
+ return true;
+}
+
+/**
+ * Perform PBO-read mapping.
+ * If any GL errors are detected, they'll be recorded and NULL returned.
+ * \sa _mesa_validate_pbo_source
+ * \sa _mesa_map_pbo_source
+ * A call to this function should have a matching call to
+ * _mesa_unmap_pbo_source().
+ */
+const GLvoid *
+_mesa_map_validate_pbo_source(struct gl_context *ctx,
+ GLuint dimensions,
+ const struct gl_pixelstore_attrib *unpack,
+ GLsizei width, GLsizei height, GLsizei depth,
+ GLenum format, GLenum type,
+ GLsizei clientMemSize,
+ c
