Re: [Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker

2011-11-27 Thread Marek Olšák
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

2011-11-27 Thread Ian Romanick

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

2011-11-26 Thread Jose Fonseca
- 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

2011-11-26 Thread Marek Olšák
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

2011-11-25 Thread Roland Scheidegger
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

2011-11-25 Thread Dave Airlie
> 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

2011-11-25 Thread 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.

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

2011-11-24 Thread Kenneth Graunke
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

2011-11-24 Thread Alan Coopersmith

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

2011-11-24 Thread Dave Airlie
> 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

2011-11-24 Thread Marek Olšák
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

2011-11-23 Thread Marek Olšák
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

2011-11-23 Thread Ian Romanick

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

2011-11-23 Thread Ian Romanick

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

2011-11-23 Thread Kenneth Graunke
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

2011-11-23 Thread Marek Olšák
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

2011-11-22 Thread Kenneth Graunke
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

2011-11-22 Thread Marek Olšák
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

2011-11-22 Thread Ian Romanick

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

2011-11-22 Thread Marek Olšák
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

2011-11-22 Thread Jose Fonseca
- 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

2011-11-22 Thread Marek Olšák
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

2011-11-22 Thread Jose Fonseca
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