Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Mon, Nov 28, 2011 at 12:08 AM, Ian Romanick wrote: > In Mesa, we have four drivers that advertise 4096 uniform components on some > combinations of hardware: i915, i915g, nouveau, and r300g. By putting > resource counting in the driver we have the potential for shaders to compile > and link on exactly one driver but fail on the other three. r300g does not advertise 4096 uniform components. The limits are: 1024 uniform components on r300 vertex shaders. 128 uniform components on r300 fragment shaders. 1024 uniform components on r500 fragment shaders. I was investigating how the Intel driver sets the maximum and I didn't find it. Maybe the Intel driver is reporting Mesa defaults (most probably incorrect). Maybe I didn't look hard enough. I have not faked any limits so far. But now I will probably have to raise them a bit in order to comply with the strict rules of the GLSL linker, to close bugs in some other way than WONTFIX of course, and just making work what used to work. Yes, it's lying, but it would make things better at the same time, what other option do I have? (with the least amount of work, of course, I have better things to do) > Which is it? Is a consistent ecosystem on which developers can depend a > virtue, or is it me trying to ruin someone's driver? You can't argue that > it's both. > > If someone thinks that compacting uniform arrays or varying arrays is an > important optimization that our OpenGL ecosystem needs to make applications > run, that's great. Patches are welcome. Great. I just committed this last year, is it welcome too? http://cgit.freedesktop.org/mesa/mesa/commit/?id=574ba4b5f50bfe661427327cd792a8a200559376 Though it seems to me that what you're telling me is "give me a patch for the GLSL IR or off". In theory, the GLSL compiler is not the only client in Gallium, so I guess none of the Gallium folks are gonna be eager to move things into it. If I move some optimization into the GLSL compiler, it doesn't mean I can remove it from a Gallium driver. I have to keep in mind there's video acceleration and maybe even X acceleration coming for Gallium. As far as varyings are concerned, no driver can do compaction to my knowledge yet. The problem with the GLSL compiler is that it can't even do a _simple_ dead-code elimination on unused varyings like gl_TexCoord[insert_literal_here], i.e. something the years-old ARB program implementation can do (and set the OutputsWritten flag accordingly). For example, if gl_TexCoord[1] is unused, what's stopping you from assigning some user-defined varying to the same location? I am not expecting an answer, I know this issue will be fixed by somebody sooner or later. I am just showing the problem. > This is an ecosystem. We will all either fail or succeed together. Why do > you think so much of the work that I and my team does lives in the shared > components? Huge amounts of the code that we've written, especially the > GLSL optimizations, could live in our drivers. Doing so would have let us > accomplish our goals much, much faster, but we didn't do that because it > hurts the ecosystem as a whole. I agree, but forcing the rules of your driver onto somebody else's driver may be met with disagreement, especially if your shader backend is mediocre to others (because it heavily relies on shared components, unlike other backends, especially Gallium drivers cannot solely rely just on the GLSL compiler). I don't think Mesa is an ecosystem to users such that it has to behave the same no matter what driver is used. Mesa competes with two proprietary drivers, which own quite a large market share, and both are by several orders of magnitude better. They are so much better and popular among professionals that they basically set the standard, because most apps are tested on them and made work on them. Who really cares about Mesa besides regular users who don't/can't replace the graphics driver (including distribution vendors) and open source enthusiasts? We can only shut up and do what the other driver providers do.* Now that I live in a world where some apps are tested only on R600-class hardware (at least) and proprietary drivers, I have to make compromises and hard decisions to keep them working on the old hardware. I tried hard to avoid reporting false limits, but I seem to have been given no other choice. * For example, I cannot advertise ARB_draw_instanced in r600g, because it would break Unigine Heaven and pretty much anything else which uses the extension. Mesa's implementation of ARB_draw_instanced is broken for me. Whether it's broken in the name of religion or compliance with the specification, I don't really give a damn. What I truly give a damn about is whether the Unigine engine works. And last time I tried, it worked without any graphical glitches on all Radeon Gallium drivers as long as I don't enable that extension. Marek ___ mesa-dev mailing list mesa-dev@lists.freede
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/25/2011 01:13 AM, Dave Airlie wrote: I'm honestly shocked to read this, Marek. We "deliberately want [your] driver to be less capable" and "couldn't care about less about anything except [our] driver"? This is sounding remarkably like a conspiracy theory, and I really didn't expect that from you. I think it takes two to tango, and Ian was pretty direct on irc, he might have been attempting sarcasm but we all know how well that works on irc :-) The IRC logs are public, but I don't expect anyone to ever try and use an argument like "I know better than you because I'm on the ARB" in any discussion around here again. Its show contempt for those of us who know that standards are joke no matter who produces them. DirectX works because it has a decent test suite + reference renderer not because it has a tight spec group. That's not at all accurate. DirectX has an extremely tight and rigorous spec: the *one* implementation of the API. Because of that, an application developer has quite a bit of assurance that a shader targeting DX 9.0c will run on all hardware if it runs on any hardware. OpenGL on Apple platforms is in a similar situation for a similar reason. There is one implementation of all of the application facing bits. If a shader compiles and links on one driver that advertises 4096 uniform components, then it will compile and link on all of them that advertise 4096 uniform components. In Mesa, we have four drivers that advertise 4096 uniform components on some combinations of hardware: i915, i915g, nouveau, and r300g. By putting resource counting in the driver we have the potential for shaders to compile and link on exactly one driver but fail on the other three. Which is it? Is a consistent ecosystem on which developers can depend a virtue, or is it me trying to ruin someone's driver? You can't argue that it's both. If someone thinks that compacting uniform arrays or varying arrays is an important optimization that our OpenGL ecosystem needs to make applications run, that's great. Patches are welcome. This is an ecosystem. We will all either fail or succeed together. Why do you think so much of the work that I and my team does lives in the shared components? Huge amounts of the code that we've written, especially the GLSL optimizations, could live in our drivers. Doing so would have let us accomplish our goals much, much faster, but we didn't do that because it hurts the ecosystem as a whole. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
- Original Message - > People are being more honest on > IRC than here. I read some of the IRC logs from the past days, and there were several saddening statements made in there by several people. But I wonder if IRC does really bring the most honest side out of people, or simply their angriest side. Either way, I'm glad I don't use IRC. I don't need the added grief, and I don't known about people in general, but I found out that my anger follows a negative exponential curve. So much that, to counteract it, whenever I get pissed at some email post I adopted a procedure: I start with a very angry draft reply, then every few hours I edit and tone down the nastier bits, till all emotion fades away. Sometimes I decide it's not even worth replying at all. Much later, I often look back, and think what would have happened if I had sent that very angry initial reply, and it is invariably followed by a big relief of not having done such foolish thing. So, it's easy to imagine the disastrous consequences of being able to quickly reply with what's on my mind at the spur of the moment, as IRC allows... FWIW, I have my share of experience on open-source / remote development, and when there is big disagreement I think that at the end of the day it boils down to an individual choice: either one vows to prove other's wrong / ill-intentioned; or one chooses that having an healthy environment is more important, so learns to put things behind ones' back, give the benefit of doubt, and to make the most out of what people/situations afford. I too often made the former choice only to know bitter disappointment and frustration. It's not worth it to me anymore. Being right is overrated. But this is a choice that each one of us has to do. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Fri, Nov 25, 2011 at 1:46 AM, Kenneth Graunke wrote: >> another, no driver should be crippled ever. I know you couldn't care >> less about anything except your driver, but there are people who care. >> >> Marek > > I'm honestly shocked to read this, Marek. We "deliberately want [your] > driver to be less capable" and "couldn't care about less about anything > except [our] driver"? This is sounding remarkably like a conspiracy > theory, and I really didn't expect that from you. > > Please take a step back and look at what you're saying. Kenneth, I wasn't talking about you specifically. You may have missed it. He said (on IRC) that he takes an exception to breaking other drivers because we wouldn't even have the GLSL compiler if it wasn't for the work Intel did. People are being more honest on IRC than here. Maybe somebody else from Intel should be put in charge of the GLSL compiler, I don't know. I step aside from the linker matter and I don't want to have anything to do with it, ever. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
Am 25.11.2011 09:15, schrieb Jose Fonseca: > - Original Message - >> While Ian and I may take a more idealist stance (favoring strict >> compliance), and you perhaps a more pragmatic view (get more apps >> running), we're all working toward the same goal: to build great >> OpenGL drivers and advance the state of the art for open source 3D >> graphics. >> >> Our community has been quite fractured for some time now, and I'd >> rather not see it devolve any worse that it has. This argument >> really isn't helping us solve anything. >> >> I made two suggestions for how we could work around the issue. >> The first was to move said optimizations into the common compiler, >> where they could benefit Radeon, Nouveau, and Intel. I believe the >> gist is that, while this is a good idea long term, it's too much >> work at the moment. >> >> The second was to move the resource limit checks into the driver >> so they would occur after any backend optimizations. I haven't >> heard from anyone whether they think this would solve the problem, >> whether they think it's a good idea, or how we might best make it >> happen. >> >> Or, perhaps someone has a better idea still. > > I think you've summarized very well my feelings and thoughts on this > as well, Kenneth. > > I think that moving resource limit checks into the driver has own > merit, not only for this case where the driver can handle shaders > that exceed limits by optimizing things away, but also the reverse > case where the driver does not handle shaders which do not exceed the > advertise limit due to workarounds -- many drivers need to emit > additional temp registers, varyings, or uniforms when translating > certain instructions/features. And applications that do check errors > would probably appreciate an error message when the driver can't > handle the shader, rather than silent corruption. > > Moving the resource limits into the driver shouldn't be hard. The > only question is how to handle shader variants due to non orthogonal > state -- probably the driver needs to do a trial compilation with a > representative/worst-case variant when the shader is linked. > > For gallium drivers there is also the issue that there is no way to > express error conditions when the pipe driver compiles shaders > (distinguish e.g. OOM or shader beyond limits) in the interface, but > this is trivial to fix. > I think it would be nice (for interface reasons) if we could do the resource checks in the glsl compiler, but as you said ultimately only the backend really knows with certainty if a shader is going to work. But due to possible non-orthogonal state the answer might not be correct neither, so it won't really solve the problem 100%. So I don't know how this should be solved, but in any case I'd vote for more optimizations being done in common code in any case. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
> I'm honestly shocked to read this, Marek. We "deliberately want [your] > driver to be less capable" and "couldn't care about less about anything > except [our] driver"? This is sounding remarkably like a conspiracy > theory, and I really didn't expect that from you. I think it takes two to tango, and Ian was pretty direct on irc, he might have been attempting sarcasm but we all know how well that works on irc :-) The IRC logs are public, but I don't expect anyone to ever try and use an argument like "I know better than you because I'm on the ARB" in any discussion around here again. Its show contempt for those of us who know that standards are joke no matter who produces them. DirectX works because it has a decent test suite + reference renderer not because it has a tight spec group. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
- Original Message - > While Ian and I may take a more idealist stance (favoring strict > compliance), and you perhaps a more pragmatic view (get more apps > running), we're all working toward the same goal: to build great > OpenGL > drivers and advance the state of the art for open source 3D graphics. > > Our community has been quite fractured for some time now, and I'd > rather > not see it devolve any worse that it has. This argument really isn't > helping us solve anything. > > I made two suggestions for how we could work around the issue. The > first was to move said optimizations into the common compiler, where > they could benefit Radeon, Nouveau, and Intel. I believe the gist is > that, while this is a good idea long term, it's too much work at the > moment. > > The second was to move the resource limit checks into the driver so > they > would occur after any backend optimizations. I haven't heard from > anyone whether they think this would solve the problem, whether they > think it's a good idea, or how we might best make it happen. > > Or, perhaps someone has a better idea still. I think you've summarized very well my feelings and thoughts on this as well, Kenneth. I think that moving resource limit checks into the driver has own merit, not only for this case where the driver can handle shaders that exceed limits by optimizing things away, but also the reverse case where the driver does not handle shaders which do not exceed the advertise limit due to workarounds -- many drivers need to emit additional temp registers, varyings, or uniforms when translating certain instructions/features. And applications that do check errors would probably appreciate an error message when the driver can't handle the shader, rather than silent corruption. Moving the resource limits into the driver shouldn't be hard. The only question is how to handle shader variants due to non orthogonal state -- probably the driver needs to do a trial compilation with a representative/worst-case variant when the shader is linked. For gallium drivers there is also the issue that there is no way to express error conditions when the pipe driver compiles shaders (distinguish e.g. OOM or shader beyond limits) in the interface, but this is trivial to fix. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/24/2011 04:47 AM, Marek Olšák wrote: > On Wed, Nov 23, 2011 at 7:25 PM, Ian Romanick wrote: >> Let me paraphrase this a little bit in a way that I think concisely captures >> the intention: >> >>"We need to work really hard to make things work on older hardware." >> >> I don't think anyone disagrees with that. However, the solutions you have >> so far proposed to this problem have said: >> >>"We need to let anything through whether it will work or not." >> >> Those are very different things. We can have the first without the second. >> I will fight very, very hard to not allow the second in any project with >> which I'm associated. > > In this specific case, as I understand it, you deliberately want my > driver to be less capable for the next release (and you probably > already managed to do that for 7.11, though I think 7.10 was okay). > What I have so far proposed is: > > We need to let *some* shaders through whether they will work or not, > because the current code is deliberately crippling at least one > driver. In order to fix this, some checks should be removed > *temporarily* and reestablished when they're more mature, or another > workaround should be found before the next release is out. One way or > another, no driver should be crippled ever. I know you couldn't care > less about anything except your driver, but there are people who care. > > Marek I'm honestly shocked to read this, Marek. We "deliberately want [your] driver to be less capable" and "couldn't care about less about anything except [our] driver"? This is sounding remarkably like a conspiracy theory, and I really didn't expect that from you. Please take a step back and look at what you're saying. I personally want to see open source 3D graphics flourish, regardless of vendor. I have the privilege of working on the Intel driver, and so I will try to make it the best I possibly can. But that doesn't mean I'm not glad to see you and others making great improvements on the Radeon and Nouveau drivers, too. Yes, we "compete" to an extent---but we also collaborate. I don't think I would accuse anyone in the community of intentionally sabotaging another person's work. I think our track record speaks differently. We've invested a lot into core Mesa and improving common infrastructure over the years. When we work on the GLSL compiler, we try to design things in a way to be helpful to driver developers across the full range of hardware, not just ours. We've also done a bunch of work on GLX, EGL, texturing, and more. You've also done a great deal of solid work in your driver, in core Mesa, and also in Piglit, and I respect that. I feel confident in saying that the whole community is grateful for your efforts. People on my team speak also very highly of you. While Ian and I may take a more idealist stance (favoring strict compliance), and you perhaps a more pragmatic view (get more apps running), we're all working toward the same goal: to build great OpenGL drivers and advance the state of the art for open source 3D graphics. Our community has been quite fractured for some time now, and I'd rather not see it devolve any worse that it has. This argument really isn't helping us solve anything. I made two suggestions for how we could work around the issue. The first was to move said optimizations into the common compiler, where they could benefit Radeon, Nouveau, and Intel. I believe the gist is that, while this is a good idea long term, it's too much work at the moment. The second was to move the resource limit checks into the driver so they would occur after any backend optimizations. I haven't heard from anyone whether they think this would solve the problem, whether they think it's a good idea, or how we might best make it happen. Or, perhaps someone has a better idea still. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/24/11 05:25, Dave Airlie wrote: On UNIX malloc generally hasn't returned NULL since overcommit was invented, what happens now is some time in the future you attempt to use a page and your app dies. s/UNIX/Linux/ - other Unixes still prefer reliability over random OOM crashes. -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
> By this same logic, malloc should never return NULL because most apps can't > handle it. Instead it should mmap /dev/null and return a pointer to that. > That analogy isn't as far off as it may seem: in both cases the underlying > infrastructure has lied to the application that an operation succeeded, and > it has given it a resource that it can't possibly use. malloc actually hardly ever returns NULL, so probably want to pick a more sane example. On UNIX malloc generally hasn't returned NULL since overcommit was invented, what happens now is some time in the future you attempt to use a page and your app dies. So not only has someone suggested this, but its been implemented to do what you least want, which is crash the app. Dave. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Wed, Nov 23, 2011 at 7:25 PM, Ian Romanick wrote: > Let me paraphrase this a little bit in a way that I think concisely captures > the intention: > > "We need to work really hard to make things work on older hardware." > > I don't think anyone disagrees with that. However, the solutions you have > so far proposed to this problem have said: > > "We need to let anything through whether it will work or not." > > Those are very different things. We can have the first without the second. > I will fight very, very hard to not allow the second in any project with > which I'm associated. In this specific case, as I understand it, you deliberately want my driver to be less capable for the next release (and you probably already managed to do that for 7.11, though I think 7.10 was okay). What I have so far proposed is: We need to let *some* shaders through whether they will work or not, because the current code is deliberately crippling at least one driver. In order to fix this, some checks should be removed *temporarily* and reestablished when they're more mature, or another workaround should be found before the next release is out. One way or another, no driver should be crippled ever. I know you couldn't care less about anything except your driver, but there are people who care. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Wed, Nov 23, 2011 at 7:27 PM, Ian Romanick wrote: > On 11/23/2011 05:42 AM, Marek Olšák wrote: >> >> On Wed, Nov 23, 2011 at 6:59 AM, Kenneth Graunke >> wrote: >>> >>> So, again, if the interest is in making more apps succeed, we should >>> start with varying packing. That's useful all around, and I doubt >>> anyone can dispute that it's necessary. >> >> No, that's not the problem. Varying packing is indeed important, but >> it's far less important that the problem this discussion is all about. >> We should start with breaking arrays into elements when possible when >> doing those checks. Consider this shader: >> >> varying vec4 array[5]; >> ... >> gl_TexCoord[7] = ...; /* adds 8*4 varyings components */ >> array[4] = ...; /* adds 5*4 varying components */ >> >> /* linker stats: 13*4 varying components used --> FAIL */ >> /* r300g stats: 2*4 components used -> PASS */ > > Do you have any concrete, real-world examples of this? If so, can you name > the applications? I'd like to add tests that model their behavior to > piglit. Civilization 4 in Wine. I think that's the kind of an app people would like to use Wine for. https://bugs.freedesktop.org/show_bug.cgi?id=43121 > > Every example that I have seen has looked more like: > > varying vec2 array[4]; // should be 8 varying components > > gl_TexCoord[2] = ...; > gl_TexCoord[5] = ...; // should be at most 24 varying components > > for (i = 0; i < array.length(); i++) > array[i] = ...; > > In this case, array gets stored as 4 vec4s using 16 varying components. As > a result, 16+24 doesn't fit when 8+24 would have. > >> It's the exact same problem with uniforms. The thing is r300g has >> these optimizations already implemented for varyings and for uniforms. >> Disabling not just one, but two optimizations at the same time just >> because they should be done in the GLSL compiler and not in the >> driver, seems quite unfriendly to me, almost like you didn't want me >> to enable them. I would probably implement them in the GLSL compiler, > > That's overstating things quite a bit. The optimizations aren't disabled. > I'm sure that r300g still gets significant performance benefits from this, > and nothing has changed that. The resource counting has always existed, so > nothing has changed there either. The elimination of unused uniforms gives no performance benefits (there is a even slight performance penalty due to a remapping table being used). The only motivation to write it was that the driver could run more Wine apps and also we were able to close some bugs with it (I guess they can be pretty much re-opened now, great). The elimination of unused varyings is a simple dead-code elimination and the outputs are packed next to each other because that's how hardware expects them to be. Also, the color varyings shouldn't contribute to the number of used varying components on some (or all?) GPUs, because there are dedicated outputs for them. I just hope you don't add the back colors too. Now back on topic. It's now clear to me that if any shader can't run, you want it to fail linking. However the r300 compiler may still fail in a number of cases, even now. Do you want to report a failure every time? If yes, then we need an interface for drivers to be able to report compile errors in a form that can be presented to the user, and at the same time the driver should be the one to decide whether a shader can be run, not some shared component. Do you have any opinions? Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/23/2011 05:42 AM, Marek Olšák wrote: On Wed, Nov 23, 2011 at 6:59 AM, Kenneth Graunke wrote: So, again, if the interest is in making more apps succeed, we should start with varying packing. That's useful all around, and I doubt anyone can dispute that it's necessary. No, that's not the problem. Varying packing is indeed important, but it's far less important that the problem this discussion is all about. We should start with breaking arrays into elements when possible when doing those checks. Consider this shader: varying vec4 array[5]; ... gl_TexCoord[7] = ...; /* adds 8*4 varyings components */ array[4] = ...; /* adds 5*4 varying components */ /* linker stats: 13*4 varying components used --> FAIL */ /* r300g stats: 2*4 components used -> PASS */ Do you have any concrete, real-world examples of this? If so, can you name the applications? I'd like to add tests that model their behavior to piglit. Every example that I have seen has looked more like: varying vec2 array[4]; // should be 8 varying components gl_TexCoord[2] = ...; gl_TexCoord[5] = ...; // should be at most 24 varying components for (i = 0; i < array.length(); i++) array[i] = ...; In this case, array gets stored as 4 vec4s using 16 varying components. As a result, 16+24 doesn't fit when 8+24 would have. It's the exact same problem with uniforms. The thing is r300g has these optimizations already implemented for varyings and for uniforms. Disabling not just one, but two optimizations at the same time just because they should be done in the GLSL compiler and not in the driver, seems quite unfriendly to me, almost like you didn't want me to enable them. I would probably implement them in the GLSL compiler, That's overstating things quite a bit. The optimizations aren't disabled. I'm sure that r300g still gets significant performance benefits from this, and nothing has changed that. The resource counting has always existed, so nothing has changed there either. say, next year (I don't and can't have deadlines), but there is no reason for me to do so, because I already have them. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/22/2011 07:27 PM, Marek Olšák wrote: On Tue, Nov 22, 2011 at 11:11 PM, Ian Romanick wrote: All of this discussion is largely moot. The failure that you're so angry about was caused by a bug in the check, not by the check itself. That bug has already been fixed (commit 151867b). The exact same check was previously performed in st_glsl_to_tgsi (or ir_to_mesa), and the exact same set of shaders would have been rejected. The check is now done in the linker instead. Actually, the bug only got my attention and then I realized what is actually happening in the linker. I probably wouldn't even notice because I no longer do any 3D on my laptop with r500. I gotta admit, I didn't know the checks were so... well, "not ready for a release" to say the least and that's meant regardless of the bug. Let's analyze the situation a bit, open-minded. The checks can be enabled for OpenGL ES 2.0 with no problem, we won't likely get a failure there. They can also be enabled for D3D10-level and later hardware, because its limits are pretty high and therefore are unlikely to fail. The problem is with the D3D9-level hardware (probably related to the vmware driver too). Let me paraphrase this a little bit in a way that I think concisely captures the intention: "We need to work really hard to make things work on older hardware." I don't think anyone disagrees with that. However, the solutions you have so far proposed to this problem have said: "We need to let anything through whether it will work or not." Those are very different things. We can have the first without the second. I will fight very, very hard to not allow the second in any project with which I'm associated. We also have to consider that a lot of applications are now developed with D3D10-level or later hardware and even though the expected hardware requirements for such an app are meant to be low, there can be, say, programming mistakes, which raise hardware requirements quite a lot. The app developer has no way to know about it, because it just works on his machine. For example, some compositing managers had such mistakes and there's been a lot of whining about that on Phoronix. We also should take into account that hardly any app has a fallback if a shader program fails to link. VDrift has one, but that's rather an exception to the rule (VDrift is an interesting example though; it falls back to fixed-function because Mesa is too strict about obeying specs, just that really). Most apps usually just abort, crash, or completely ignore that linking failed and render garbage or nothing. Wine, our biggest user of Mesa, can't fail. D3D shaders must compile successfully or it's game over. Here's the deal about Wine and compositing (my spell checker always wants to make that word "composting") managers. All of the closed-source driver makers have developer outreach programs that work closely with tier-1 developers to make sure their apps work and run well. This is how they avoid a lot of these sorts of problems. It's unreasonable to expect any developer to test their product on every piece of hardware. We (the Mesa community) can't even manage that with our drivers. What we can do is try to prevent app developers from shooting themselves in the foot. We've had a lot more communication with the Wine developers over the last year or so, and things have gotten a lot better there. We can and should be more proactive, but I'm not sure what form that should take. Pretty much everything we do with app developers is reactive. Right? We only interact with them when they come to us because something doesn't work or they got a bug report from a user. Although the possibility of a linker failure is a nice feature in theory, the reality is nobody wants it, because it's the primary cause of apps aborting themselves or just rendering nothing (and, of course, everybody blames Mesa, or worse: Linux). By this same logic, malloc should never return NULL because most apps can't handle it. Instead it should mmap /dev/null and return a pointer to that. That analogy isn't as far off as it may seem: in both cases the underlying infrastructure has lied to the application that an operation succeeded, and it has given it a resource that it can't possibly use. Surely nobody would suggest glibc implement such a thing much less implement it. We shouldn't either. Both cases may make some deployed application run. However, what happens to the poor schmuck writing an application that accidentally tries 5TB instead of 5MB? He spends hours trying to figure out why all his reads of malloc'ed memory give zeros. My day job is writing OpenGL drivers. My evening job is teaching people how to write OpenGL applications. I have seen people try to debug OpenGL code, and it's already a miserable process. There are so many things that can lead to a mysterious black screen. Adding another by lying to the developer doesn't
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/23/2011 05:42 AM, Marek Olšák wrote: > On Wed, Nov 23, 2011 at 6:59 AM, Kenneth Graunke > wrote: >> On 11/22/2011 07:27 PM, Marek Olšák wrote: >>> On Tue, Nov 22, 2011 at 11:11 PM, Ian Romanick wrote: All of this discussion is largely moot. The failure that you're so angry about was caused by a bug in the check, not by the check itself. That bug has already been fixed (commit 151867b). The exact same check was previously performed in st_glsl_to_tgsi (or ir_to_mesa), and the exact same set of shaders would have been rejected. The check is now done in the linker instead. >>> >>> Actually, the bug only got my attention and then I realized what is >>> actually happening in the linker. I probably wouldn't even notice >>> because I no longer do any 3D on my laptop with r500. I gotta admit, I >>> didn't know the checks were so... well, "not ready for a release" to >>> say the least and that's meant regardless of the bug. >> >> Well. Whether they're "ready for a release" or not, the truth is that >> we've been shipping them in releases for quite some time. So we haven't >> regressed anything; it already didn't work. >> >> I am fully in support of doing additional optimizations in the compiler >> to reduce resource usage and make more applications run on older >> hardware. Splitting uniform arrays and pruning unused sections could >> definitely happen in the compiler, which as an added bonus, makes the >> optimization available on all backends. It would also eliminate unused >> resources prior to the link-time checks. >> >> Also, we desperately need to pack varyings. Currently, we don't pack >> them at all, which is both severely non-spec-compliant and very likely >> to break real world applications. We can all agree on this, so let's >> start here. Varyings are precious resources even on modern GPUs. >> >> However, I cannot in good conscience just disable resource checking >> altogether (which is what I believe you're proposing). There are so >> many cases where applications could _actually need_ more resources than >> the hardware supports, and in that case, giving an error is the only >> sensible option. What else would you do? Crash, render nothing, >> replace those uniforms/varyings with zero and render garbage? Those are >> the very behaviors you're trying to avoid. >> >> By giving an error, the application at least has the -chance- to try and >> drop down from its "High Quality" shaders to Medium/Low quality settings >> for older cards. I know many apps don't, but some do, so we should give >> them the chance. >> >> There has to be resource checking somewhere. Perhaps the backend is a >> more suitable place than the linker; I don't know. (I can see wanting >> to move optimizations before checks or checks after optimizations...) >> But we can't just remove checking entirely; that's just broken. >> >>> Let's analyze the situation a bit, open-minded. >>> >>> The checks can be enabled for OpenGL ES 2.0 with no problem, we won't >>> likely get a failure there. >>> >>> They can also be enabled for D3D10-level and later hardware, because >>> its limits are pretty high and therefore are unlikely to fail. The >>> problem is with the D3D9-level hardware (probably related to the >>> vmware driver too). >>> >>> We also have to consider that a lot of applications are now developed >>> with D3D10-level or later hardware and even though the expected >>> hardware requirements for such an app are meant to be low, there can >>> be, say, programming mistakes, which raise hardware requirements quite >>> a lot. The app developer has no way to know about it, because it just >>> works on his machine. For example, some compositing managers had such >>> mistakes and there's been a lot of whining about that on Phoronix. >> >> And returning an decent error makes the mistake abundandly clear to the >> application developer: "I used...wait, 500 of those? That can't be >> right." To use your example of compositors (typically open source), the >> compositor may not run for some people, but the application developer >> can fix it. >> >> In contrast, incorrect rendering provides _no_ useful information and >> will likely make said application developer think it's a driver bug, >> blame Mesa, and not even bother to investigate further. >> >>> We also should take into account that hardly any app has a fallback if >>> a shader program fails to link. VDrift has one, but that's rather an >>> exception to the rule (VDrift is an interesting example though; it >>> falls back to fixed-function because Mesa is too strict about obeying >>> specs, just that really). Most apps usually just abort, crash, or >>> completely ignore that linking failed and render garbage or nothing. >>> Wine, our biggest user of Mesa, can't fail. D3D shaders must compile >>> successfully or it's game over. >>> >>> Although the possibility of a linker failure is a nice feature in >>> theory, the reality is nobody w
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Wed, Nov 23, 2011 at 6:59 AM, Kenneth Graunke wrote: > On 11/22/2011 07:27 PM, Marek Olšák wrote: >> On Tue, Nov 22, 2011 at 11:11 PM, Ian Romanick wrote: >>> All of this discussion is largely moot. The failure that you're so angry >>> about was caused by a bug in the check, not by the check itself. That bug >>> has already been fixed (commit 151867b). >>> >>> The exact same check was previously performed in st_glsl_to_tgsi (or >>> ir_to_mesa), and the exact same set of shaders would have been rejected. >>> The check is now done in the linker instead. >> >> Actually, the bug only got my attention and then I realized what is >> actually happening in the linker. I probably wouldn't even notice >> because I no longer do any 3D on my laptop with r500. I gotta admit, I >> didn't know the checks were so... well, "not ready for a release" to >> say the least and that's meant regardless of the bug. > > Well. Whether they're "ready for a release" or not, the truth is that > we've been shipping them in releases for quite some time. So we haven't > regressed anything; it already didn't work. > > I am fully in support of doing additional optimizations in the compiler > to reduce resource usage and make more applications run on older > hardware. Splitting uniform arrays and pruning unused sections could > definitely happen in the compiler, which as an added bonus, makes the > optimization available on all backends. It would also eliminate unused > resources prior to the link-time checks. > > Also, we desperately need to pack varyings. Currently, we don't pack > them at all, which is both severely non-spec-compliant and very likely > to break real world applications. We can all agree on this, so let's > start here. Varyings are precious resources even on modern GPUs. > > However, I cannot in good conscience just disable resource checking > altogether (which is what I believe you're proposing). There are so > many cases where applications could _actually need_ more resources than > the hardware supports, and in that case, giving an error is the only > sensible option. What else would you do? Crash, render nothing, > replace those uniforms/varyings with zero and render garbage? Those are > the very behaviors you're trying to avoid. > > By giving an error, the application at least has the -chance- to try and > drop down from its "High Quality" shaders to Medium/Low quality settings > for older cards. I know many apps don't, but some do, so we should give > them the chance. > > There has to be resource checking somewhere. Perhaps the backend is a > more suitable place than the linker; I don't know. (I can see wanting > to move optimizations before checks or checks after optimizations...) > But we can't just remove checking entirely; that's just broken. > >> Let's analyze the situation a bit, open-minded. >> >> The checks can be enabled for OpenGL ES 2.0 with no problem, we won't >> likely get a failure there. >> >> They can also be enabled for D3D10-level and later hardware, because >> its limits are pretty high and therefore are unlikely to fail. The >> problem is with the D3D9-level hardware (probably related to the >> vmware driver too). >> >> We also have to consider that a lot of applications are now developed >> with D3D10-level or later hardware and even though the expected >> hardware requirements for such an app are meant to be low, there can >> be, say, programming mistakes, which raise hardware requirements quite >> a lot. The app developer has no way to know about it, because it just >> works on his machine. For example, some compositing managers had such >> mistakes and there's been a lot of whining about that on Phoronix. > > And returning an decent error makes the mistake abundandly clear to the > application developer: "I used...wait, 500 of those? That can't be > right." To use your example of compositors (typically open source), the > compositor may not run for some people, but the application developer > can fix it. > > In contrast, incorrect rendering provides _no_ useful information and > will likely make said application developer think it's a driver bug, > blame Mesa, and not even bother to investigate further. > >> We also should take into account that hardly any app has a fallback if >> a shader program fails to link. VDrift has one, but that's rather an >> exception to the rule (VDrift is an interesting example though; it >> falls back to fixed-function because Mesa is too strict about obeying >> specs, just that really). Most apps usually just abort, crash, or >> completely ignore that linking failed and render garbage or nothing. >> Wine, our biggest user of Mesa, can't fail. D3D shaders must compile >> successfully or it's game over. >> >> Although the possibility of a linker failure is a nice feature in >> theory, the reality is nobody wants it, because it's the primary cause >> of apps aborting themselves or just rendering nothing (and, of course, >> everybody blames Mes
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/22/2011 07:27 PM, Marek Olšák wrote: > On Tue, Nov 22, 2011 at 11:11 PM, Ian Romanick wrote: >> All of this discussion is largely moot. The failure that you're so angry >> about was caused by a bug in the check, not by the check itself. That bug >> has already been fixed (commit 151867b). >> >> The exact same check was previously performed in st_glsl_to_tgsi (or >> ir_to_mesa), and the exact same set of shaders would have been rejected. >> The check is now done in the linker instead. > > Actually, the bug only got my attention and then I realized what is > actually happening in the linker. I probably wouldn't even notice > because I no longer do any 3D on my laptop with r500. I gotta admit, I > didn't know the checks were so... well, "not ready for a release" to > say the least and that's meant regardless of the bug. Well. Whether they're "ready for a release" or not, the truth is that we've been shipping them in releases for quite some time. So we haven't regressed anything; it already didn't work. I am fully in support of doing additional optimizations in the compiler to reduce resource usage and make more applications run on older hardware. Splitting uniform arrays and pruning unused sections could definitely happen in the compiler, which as an added bonus, makes the optimization available on all backends. It would also eliminate unused resources prior to the link-time checks. Also, we desperately need to pack varyings. Currently, we don't pack them at all, which is both severely non-spec-compliant and very likely to break real world applications. We can all agree on this, so let's start here. Varyings are precious resources even on modern GPUs. However, I cannot in good conscience just disable resource checking altogether (which is what I believe you're proposing). There are so many cases where applications could _actually need_ more resources than the hardware supports, and in that case, giving an error is the only sensible option. What else would you do? Crash, render nothing, replace those uniforms/varyings with zero and render garbage? Those are the very behaviors you're trying to avoid. By giving an error, the application at least has the -chance- to try and drop down from its "High Quality" shaders to Medium/Low quality settings for older cards. I know many apps don't, but some do, so we should give them the chance. There has to be resource checking somewhere. Perhaps the backend is a more suitable place than the linker; I don't know. (I can see wanting to move optimizations before checks or checks after optimizations...) But we can't just remove checking entirely; that's just broken. > Let's analyze the situation a bit, open-minded. > > The checks can be enabled for OpenGL ES 2.0 with no problem, we won't > likely get a failure there. > > They can also be enabled for D3D10-level and later hardware, because > its limits are pretty high and therefore are unlikely to fail. The > problem is with the D3D9-level hardware (probably related to the > vmware driver too). > > We also have to consider that a lot of applications are now developed > with D3D10-level or later hardware and even though the expected > hardware requirements for such an app are meant to be low, there can > be, say, programming mistakes, which raise hardware requirements quite > a lot. The app developer has no way to know about it, because it just > works on his machine. For example, some compositing managers had such > mistakes and there's been a lot of whining about that on Phoronix. And returning an decent error makes the mistake abundandly clear to the application developer: "I used...wait, 500 of those? That can't be right." To use your example of compositors (typically open source), the compositor may not run for some people, but the application developer can fix it. In contrast, incorrect rendering provides _no_ useful information and will likely make said application developer think it's a driver bug, blame Mesa, and not even bother to investigate further. > We also should take into account that hardly any app has a fallback if > a shader program fails to link. VDrift has one, but that's rather an > exception to the rule (VDrift is an interesting example though; it > falls back to fixed-function because Mesa is too strict about obeying > specs, just that really). Most apps usually just abort, crash, or > completely ignore that linking failed and render garbage or nothing. > Wine, our biggest user of Mesa, can't fail. D3D shaders must compile > successfully or it's game over. > > Although the possibility of a linker failure is a nice feature in > theory, the reality is nobody wants it, because it's the primary cause > of apps aborting themselves or just rendering nothing (and, of course, > everybody blames Mesa, or worse: Linux). > > There is a quite a large possibility that if those linker checks were > disabled, more apps would work, especially those were the limits are > exceeded by
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Tue, Nov 22, 2011 at 11:11 PM, Ian Romanick wrote: > All of this discussion is largely moot. The failure that you're so angry > about was caused by a bug in the check, not by the check itself. That bug > has already been fixed (commit 151867b). > > The exact same check was previously performed in st_glsl_to_tgsi (or > ir_to_mesa), and the exact same set of shaders would have been rejected. > The check is now done in the linker instead. Actually, the bug only got my attention and then I realized what is actually happening in the linker. I probably wouldn't even notice because I no longer do any 3D on my laptop with r500. I gotta admit, I didn't know the checks were so... well, "not ready for a release" to say the least and that's meant regardless of the bug. Let's analyze the situation a bit, open-minded. The checks can be enabled for OpenGL ES 2.0 with no problem, we won't likely get a failure there. They can also be enabled for D3D10-level and later hardware, because its limits are pretty high and therefore are unlikely to fail. The problem is with the D3D9-level hardware (probably related to the vmware driver too). We also have to consider that a lot of applications are now developed with D3D10-level or later hardware and even though the expected hardware requirements for such an app are meant to be low, there can be, say, programming mistakes, which raise hardware requirements quite a lot. The app developer has no way to know about it, because it just works on his machine. For example, some compositing managers had such mistakes and there's been a lot of whining about that on Phoronix. We also should take into account that hardly any app has a fallback if a shader program fails to link. VDrift has one, but that's rather an exception to the rule (VDrift is an interesting example though; it falls back to fixed-function because Mesa is too strict about obeying specs, just that really). Most apps usually just abort, crash, or completely ignore that linking failed and render garbage or nothing. Wine, our biggest user of Mesa, can't fail. D3D shaders must compile successfully or it's game over. Although the possibility of a linker failure is a nice feature in theory, the reality is nobody wants it, because it's the primary cause of apps aborting themselves or just rendering nothing (and, of course, everybody blames Mesa, or worse: Linux). There is a quite a large possibility that if those linker checks were disabled, more apps would work, especially those were the limits are exceeded by a little bit, but the difference is eliminated by the driver. Sure, some apps would still be broken or render garbage, but it's either this or nothing, don't you think? Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On 11/22/2011 07:54 AM, Marek Olšák wrote:
On Tue, Nov 22, 2011 at 4:05 PM, Jose Fonseca wrote:
Marek,
I think we should take one of two approaches here:
- aim for a short-term workaround, without gallium interface changes:
- e.g., provide to GLSL compiler infinite/huge limits, while advertising to
the app the pipe driver ones
- or detect the wine process and advertise bigger limits in the r300 driver
- aim for accurately describing the pipe driver compiling abilities
- e.g., allow the state tracker to create shaders that exceed abilities, and
force a trial generation and compilation of the TGSI shaders when the GLSL
shader is first linked
But the hard limit caps interface change you propose below doesn't quite align
to neither approach: it is an interface change which doesn't truly help
describing the pipe driver compiler abilities any better (the actual maximum
constants/inputs depends on the shader and is not a global constant), so it
merely provides a short term relief.
All of this discussion is largely moot. The failure that you're so
angry about was caused by a bug in the check, not by the check itself.
That bug has already been fixed (commit 151867b).
The exact same check was previously performed in st_glsl_to_tgsi (or
ir_to_mesa), and the exact same set of shaders would have been rejected.
The check is now done in the linker instead.
I would gladly commit this patch:
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3527088..4e1e648 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1813,6 +1813,9 @@ assign_varying_locations(struct gl_context *ctx,
}
}
+ /* Enable this once the GLSL compiler can properly count in just
+* active array elements, not whole arrays. */
+#if 0
if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
if (varying_vectors> ctx->Const.MaxVarying) {
linker_error(prog, "shader uses too many varying vectors "
@@ -1829,6 +1832,7 @@ assign_varying_locations(struct gl_context *ctx,
return false;
}
}
+#endif
return true;
}
@@ -1959,10 +1963,16 @@ check_resources(struct gl_context *ctx, struct
gl_shader_program *prog)
shader_names[i]);
}
+ /* Enable this once the GLSL compiler can properly count in just
+ * active array elements, not whole arrays. */
+#if 0
if (sh->num_uniform_components> max_uniform_components[i]) {
linker_error(prog, "Too many %s shader uniform components",
shader_names[i]);
}
+#else
+ (void) max_uniform_components;
+#endif
}
return prog->LinkStatus;
BUT I had a very informative discussion with one Intel developer last
night and he said we can't change the linker because he's on the ARB.
:D
Now seriously. I do really care about users. And I'd like to release a
product which works best for them. Some people obviously don't want me
to release such a product.
For every shader these patches would allow to run correctly, I can find
a shader that it will allow that won't (and can't) run correctly. I can
also find another driver that advertises the same limits where shaders
in the first group already don't run.
For example, these patches would allow the following shader, which
cannot possibly work correctly:
#version 120
uniform float a[gl_MaxUniformComponents + 10];
uniform int i = gl_MaxUniformComponents + 5;
void main()
{
gl_Position = vec4(a[i]);
}
This shader cannot work, and we've lost the ability to tell the user
that it cannot work. Instead of an nice error message about exceeding
limits, the user gets incorrect rendering, an assertion failure, or a
GPU hang. None of these is correct (for any definition of correct) or
what the user wants. They're also really hard for anyone (driver
developer or app developer) to debug or fix.
I'm fairly sure that such a change would cause at least one or two
OpenGL ES 2.0 conformance tests to fail. There are a lot of tests there
that poke at various corner cases of limits.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Tue, Nov 22, 2011 at 6:04 PM, Jose Fonseca wrote: > - Original Message - >> On Tue, Nov 22, 2011 at 4:05 PM, Jose Fonseca >> wrote: >> > Marek, >> > >> > I think we should take one of two approaches here: >> > - aim for a short-term workaround, without gallium interface >> > changes: >> > - e.g., provide to GLSL compiler infinite/huge limits, while >> > advertising to the app the pipe driver ones >> > - or detect the wine process and advertise bigger limits in the >> > r300 driver >> > - aim for accurately describing the pipe driver compiling abilities >> > - e.g., allow the state tracker to create shaders that exceed >> > abilities, and force a trial generation and compilation of the >> > TGSI shaders when the GLSL shader is first linked >> > >> > But the hard limit caps interface change you propose below doesn't >> > quite align to neither approach: it is an interface change which >> > doesn't truly help describing the pipe driver compiler abilities >> > any better (the actual maximum constants/inputs depends on the >> > shader and is not a global constant), so it merely provides a >> > short term relief. >> >> I would gladly commit this patch: >> > [...] > > Isn't there anything equally easy that can be done without touching > src/glsl/*? I don't think so. From the looks of it, drivers don't even get to see any shader the linker rejects. I guess it would be cleaner to have boolean flags SkipVaryingLimitChecking and SkipUniformLimitChecking in gl_constants and changing the linker to read those flags. Does that sound like a good idea? > >> Now seriously. I do really care about users. And I'd like to release >> a >> product which works best for them. > > I understand. And I agree that on this particular case this is an intolerable > regression for the users concerned. > > However, I don't think that Ian doesn't care about users, but rather that not > following the spec is more harmful to the users on the long term. And, given > he's one of the main GLSL author/maintainer that carries weight on my book, > as far as that sub-component is concerned. > > Anyway, I don't see any need for us to clash against each others: all Mesa > developers have and will always have different goals, schedules, and external > pressures; but it should be possible to share the same code between all > drivers without having to share the same policy. > > GLSL is a sub-component among others. Surely it must be possible to > accommodate both a strict enforcement of max uniforms on Intel drivers, and a > more lenient enforcement on another drivers. So instead of trying to decide > who's right, let's just focus our energy into finding that flexible solution. Sorry, I didn't mean to offend anybody. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
- Original Message - > On Tue, Nov 22, 2011 at 4:05 PM, Jose Fonseca > wrote: > > Marek, > > > > I think we should take one of two approaches here: > > - aim for a short-term workaround, without gallium interface > > changes: > > - e.g., provide to GLSL compiler infinite/huge limits, while > > advertising to the app the pipe driver ones > > - or detect the wine process and advertise bigger limits in the > > r300 driver > > - aim for accurately describing the pipe driver compiling abilities > > - e.g., allow the state tracker to create shaders that exceed > > abilities, and force a trial generation and compilation of the > > TGSI shaders when the GLSL shader is first linked > > > > But the hard limit caps interface change you propose below doesn't > > quite align to neither approach: it is an interface change which > > doesn't truly help describing the pipe driver compiler abilities > > any better (the actual maximum constants/inputs depends on the > > shader and is not a global constant), so it merely provides a > > short term relief. > > I would gladly commit this patch: > [...] Isn't there anything equally easy that can be done without touching src/glsl/*? > Now seriously. I do really care about users. And I'd like to release > a > product which works best for them. I understand. And I agree that on this particular case this is an intolerable regression for the users concerned. However, I don't think that Ian doesn't care about users, but rather that not following the spec is more harmful to the users on the long term. And, given he's one of the main GLSL author/maintainer that carries weight on my book, as far as that sub-component is concerned. Anyway, I don't see any need for us to clash against each others: all Mesa developers have and will always have different goals, schedules, and external pressures; but it should be possible to share the same code between all drivers without having to share the same policy. GLSL is a sub-component among others. Surely it must be possible to accommodate both a strict enforcement of max uniforms on Intel drivers, and a more lenient enforcement on another drivers. So instead of trying to decide who's right, let's just focus our energy into finding that flexible solution. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
On Tue, Nov 22, 2011 at 4:05 PM, Jose Fonseca wrote:
> Marek,
>
> I think we should take one of two approaches here:
> - aim for a short-term workaround, without gallium interface changes:
> - e.g., provide to GLSL compiler infinite/huge limits, while advertising to
> the app the pipe driver ones
> - or detect the wine process and advertise bigger limits in the r300 driver
> - aim for accurately describing the pipe driver compiling abilities
> - e.g., allow the state tracker to create shaders that exceed abilities, and
> force a trial generation and compilation of the TGSI shaders when the GLSL
> shader is first linked
>
> But the hard limit caps interface change you propose below doesn't quite
> align to neither approach: it is an interface change which doesn't truly help
> describing the pipe driver compiler abilities any better (the actual maximum
> constants/inputs depends on the shader and is not a global constant), so it
> merely provides a short term relief.
I would gladly commit this patch:
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3527088..4e1e648 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1813,6 +1813,9 @@ assign_varying_locations(struct gl_context *ctx,
}
}
+ /* Enable this once the GLSL compiler can properly count in just
+* active array elements, not whole arrays. */
+#if 0
if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
if (varying_vectors > ctx->Const.MaxVarying) {
linker_error(prog, "shader uses too many varying vectors "
@@ -1829,6 +1832,7 @@ assign_varying_locations(struct gl_context *ctx,
return false;
}
}
+#endif
return true;
}
@@ -1959,10 +1963,16 @@ check_resources(struct gl_context *ctx, struct
gl_shader_program *prog)
shader_names[i]);
}
+ /* Enable this once the GLSL compiler can properly count in just
+ * active array elements, not whole arrays. */
+#if 0
if (sh->num_uniform_components > max_uniform_components[i]) {
linker_error(prog, "Too many %s shader uniform components",
shader_names[i]);
}
+#else
+ (void) max_uniform_components;
+#endif
}
return prog->LinkStatus;
BUT I had a very informative discussion with one Intel developer last
night and he said we can't change the linker because he's on the ARB.
:D
Now seriously. I do really care about users. And I'd like to release a
product which works best for them. Some people obviously don't want me
to release such a product.
Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
Marek,
I think we should take one of two approaches here:
- aim for a short-term workaround, without gallium interface changes:
- e.g., provide to GLSL compiler infinite/huge limits, while advertising to
the app the pipe driver ones
- or detect the wine process and advertise bigger limits in the r300 driver
- aim for accurately describing the pipe driver compiling abilities
- e.g., allow the state tracker to create shaders that exceed abilities, and
force a trial generation and compilation of the TGSI shaders when the GLSL
shader is first linked
But the hard limit caps interface change you propose below doesn't quite align
to neither approach: it is an interface change which doesn't truly help
describing the pipe driver compiler abilities any better (the actual maximum
constants/inputs depends on the shader and is not a global constant), so it
merely provides a short term relief.
Jose
- Original Message -
> These are different from hardware limits, because some drivers can
> correctly
> handle higher values than they report in most cases, either due
> to driver-specific compiler optimizations, or due to just how the
> hardware
> works internally.
>
> The problem with the linker is that it artifically reduces driver
> capabilities
> by failing to link shaders which would otherwise run correctly.
>
> This is a regression fix.
>
> Another solution would be to just disable the broken checks in the
> linker,
> but some people obviously want to preserve the new broken behavior.
>
> NOTE: Of course, the Gallium drivers will be modified such that
> MAX_xxx == MAX_xxx_HARD_LIMIT.
> ---
> src/gallium/include/pipe/p_defines.h |4 +++-
> src/glsl/linker.cpp | 12 ++--
> src/glsl/standalone_scaffolding.cpp |3 +++
> src/mesa/drivers/dri/i915/i915_context.c |2 +-
> src/mesa/main/context.c |5 -
> src/mesa/main/mtypes.h |6 ++
> src/mesa/state_tracker/st_extensions.c |8
> 7 files changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_defines.h
> b/src/gallium/include/pipe/p_defines.h
> index 6d6faab..108830c 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -502,7 +502,9 @@ enum pipe_shader_cap
> PIPE_SHADER_CAP_SUBROUTINES = 16, /* BGNSUB, ENDSUB, CAL, RET */
> PIPE_SHADER_CAP_INTEGERS = 17,
> PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS = 18,
> - PIPE_SHADER_CAP_OUTPUT_READ = 19
> + PIPE_SHADER_CAP_OUTPUT_READ = 19,
> + PIPE_SHADER_CAP_MAX_INPUTS_HARD_LIMIT = 20,
> + PIPE_SHADER_CAP_MAX_CONSTS_HARD_LIMIT = 21
> };
>
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 3527088..77b50d8 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1814,18 +1814,18 @@ assign_varying_locations(struct gl_context
> *ctx,
> }
>
> if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
> - if (varying_vectors > ctx->Const.MaxVarying) {
> + if (varying_vectors > ctx->Const.MaxVaryingHardLimit) {
>linker_error(prog, "shader uses too many varying vectors "
> "(%u > %u)\n",
> - varying_vectors, ctx->Const.MaxVarying);
> + varying_vectors, ctx->Const.MaxVaryingHardLimit);
>return false;
>}
> } else {
>const unsigned float_components = varying_vectors * 4;
> - if (float_components > ctx->Const.MaxVarying * 4) {
> + if (float_components > ctx->Const.MaxVaryingHardLimit * 4) {
>linker_error(prog, "shader uses too many varying components "
> "(%u > %u)\n",
> - float_components, ctx->Const.MaxVarying * 4);
> + float_components, ctx->Const.MaxVaryingHardLimit * 4);
>return false;
>}
> }
> @@ -1943,8 +1943,8 @@ check_resources(struct gl_context *ctx, struct
> gl_shader_program *prog)
> };
>
> const unsigned max_uniform_components[MESA_SHADER_TYPES] = {
> - ctx->Const.VertexProgram.MaxUniformComponents,
> - ctx->Const.FragmentProgram.MaxUniformComponents,
> + ctx->Const.VertexProgram.MaxUniformComponentsHardLimit,
> + ctx->Const.FragmentProgram.MaxUniformComponentsHardLimit,
>0 /* FINISHME: Geometry shaders. */
> };
>
> diff --git a/src/glsl/standalone_scaffolding.cpp
> b/src/glsl/standalone_scaffolding.cpp
> index 24cc64a..7d51c55 100644
> --- a/src/glsl/standalone_scaffolding.cpp
> +++ b/src/glsl/standalone_scaffolding.cpp
> @@ -83,11 +83,14 @@ void initialize_context_to_defaults(struct
> gl_context *ctx, gl_api api)
> ctx->Const.MaxTextureCoordUnits = 2;
> ctx->Const.VertexProgram.MaxAttribs = 16;
>
> + ctx->Const.VertexProgram.MaxUniformComponentsHardLimit =
> ctx->Const.VertexProgram.MaxUniformComponents = 512;
> + ctx->Const.MaxVaryingHardLimit =
> ctx->Const.MaxVarying = 8; /* == gl_MaxVa
[Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker
These are different from hardware limits, because some drivers can correctly
handle higher values than they report in most cases, either due
to driver-specific compiler optimizations, or due to just how the hardware
works internally.
The problem with the linker is that it artifically reduces driver capabilities
by failing to link shaders which would otherwise run correctly.
This is a regression fix.
Another solution would be to just disable the broken checks in the linker,
but some people obviously want to preserve the new broken behavior.
NOTE: Of course, the Gallium drivers will be modified such that
MAX_xxx == MAX_xxx_HARD_LIMIT.
---
src/gallium/include/pipe/p_defines.h |4 +++-
src/glsl/linker.cpp | 12 ++--
src/glsl/standalone_scaffolding.cpp |3 +++
src/mesa/drivers/dri/i915/i915_context.c |2 +-
src/mesa/main/context.c |5 -
src/mesa/main/mtypes.h |6 ++
src/mesa/state_tracker/st_extensions.c |8
7 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/src/gallium/include/pipe/p_defines.h
b/src/gallium/include/pipe/p_defines.h
index 6d6faab..108830c 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -502,7 +502,9 @@ enum pipe_shader_cap
PIPE_SHADER_CAP_SUBROUTINES = 16, /* BGNSUB, ENDSUB, CAL, RET */
PIPE_SHADER_CAP_INTEGERS = 17,
PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS = 18,
- PIPE_SHADER_CAP_OUTPUT_READ = 19
+ PIPE_SHADER_CAP_OUTPUT_READ = 19,
+ PIPE_SHADER_CAP_MAX_INPUTS_HARD_LIMIT = 20,
+ PIPE_SHADER_CAP_MAX_CONSTS_HARD_LIMIT = 21
};
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3527088..77b50d8 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1814,18 +1814,18 @@ assign_varying_locations(struct gl_context *ctx,
}
if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
- if (varying_vectors > ctx->Const.MaxVarying) {
+ if (varying_vectors > ctx->Const.MaxVaryingHardLimit) {
linker_error(prog, "shader uses too many varying vectors "
"(%u > %u)\n",
- varying_vectors, ctx->Const.MaxVarying);
+ varying_vectors, ctx->Const.MaxVaryingHardLimit);
return false;
}
} else {
const unsigned float_components = varying_vectors * 4;
- if (float_components > ctx->Const.MaxVarying * 4) {
+ if (float_components > ctx->Const.MaxVaryingHardLimit * 4) {
linker_error(prog, "shader uses too many varying components "
"(%u > %u)\n",
- float_components, ctx->Const.MaxVarying * 4);
+ float_components, ctx->Const.MaxVaryingHardLimit * 4);
return false;
}
}
@@ -1943,8 +1943,8 @@ check_resources(struct gl_context *ctx, struct
gl_shader_program *prog)
};
const unsigned max_uniform_components[MESA_SHADER_TYPES] = {
- ctx->Const.VertexProgram.MaxUniformComponents,
- ctx->Const.FragmentProgram.MaxUniformComponents,
+ ctx->Const.VertexProgram.MaxUniformComponentsHardLimit,
+ ctx->Const.FragmentProgram.MaxUniformComponentsHardLimit,
0 /* FINISHME: Geometry shaders. */
};
diff --git a/src/glsl/standalone_scaffolding.cpp
b/src/glsl/standalone_scaffolding.cpp
index 24cc64a..7d51c55 100644
--- a/src/glsl/standalone_scaffolding.cpp
+++ b/src/glsl/standalone_scaffolding.cpp
@@ -83,11 +83,14 @@ void initialize_context_to_defaults(struct gl_context *ctx,
gl_api api)
ctx->Const.MaxTextureCoordUnits = 2;
ctx->Const.VertexProgram.MaxAttribs = 16;
+ ctx->Const.VertexProgram.MaxUniformComponentsHardLimit =
ctx->Const.VertexProgram.MaxUniformComponents = 512;
+ ctx->Const.MaxVaryingHardLimit =
ctx->Const.MaxVarying = 8; /* == gl_MaxVaryingFloats / 4 */
ctx->Const.MaxVertexTextureImageUnits = 0;
ctx->Const.MaxCombinedTextureImageUnits = 2;
ctx->Const.MaxTextureImageUnits = 2;
+ ctx->Const.FragmentProgram.MaxUniformComponentsHardLimit =
ctx->Const.FragmentProgram.MaxUniformComponents = 64;
ctx->Const.MaxDrawBuffers = 1;
diff --git a/src/mesa/drivers/dri/i915/i915_context.c
b/src/mesa/drivers/dri/i915/i915_context.c
index ca03ad0..0fab9d2 100644
--- a/src/mesa/drivers/dri/i915/i915_context.c
+++ b/src/mesa/drivers/dri/i915/i915_context.c
@@ -131,7 +131,7 @@ i915CreateContext(int api,
ctx->Const.MaxTextureUnits = I915_TEX_UNITS;
ctx->Const.MaxTextureImageUnits = I915_TEX_UNITS;
ctx->Const.MaxTextureCoordUnits = I915_TEX_UNITS;
- ctx->Const.MaxVarying = I915_TEX_UNITS;
+ ctx->Const.MaxVaryingHardLimit = ctx->Const.MaxVarying = I915_TEX_UNITS;
ctx->Const.MaxCombinedTextureImageUnits =
ctx->Const.MaxVertexTextureImageUnits +
ctx->Const.MaxTextureImageUnits;
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index e0af6ee..75b3441 100644
--- a/src/mesa/main/context.c
++
