[Mesa-dev] [PATCH] mesa: Define helper function to get the number of texture layers.

[Francisco Jerez]

Brian Paul <brianp at vmware.com> writes:

> On 12/10/2013 03:27 PM, Francisco Jerez wrote:
>> Brian Paul <brianp at vmware.com> writes:
>>[...]
>>> I'm more convinced now that we should simply have an assert there rather
>>> than unreachable().
>>>
>>> Finally, in release builds we generally don't want to crash in these
>>> situations.  So IMHO, it's preferable to do something like:
>>>
>> I have the feeling that this is an orthogonal question to unreachable()
>> aborting or not.  I'm not convinced that there's a "sensible" default in
>> this case, and setting one seems like trying to delay the inevitable to
>> me.  An invariant of the program has been broken and it's likely that
>> something will go very wrong sooner or later.  The question boils down
>> to: do we want undefined behavior now, or later?  I'd rather have it
>> now, but it seems like a moot question to me...
>
> In the cases at hand, _mesa_tex_target_is_layered() and 
> _mesa_get_texture_layers(), returning false/0 seem like fairly 
> reasonable defaults to return and hopefully wouldn't lead to a later crash.
>
> Over the years I've seen several bug reports where someone runs their 
> app and sees a bunch of _mesa_problem() error messages but the app keeps 
> running and doesn't crash.  I think that's preferable to just crashing 
> (esp. when Mesa may be running in the X server).  That's the reason I'm 
> hesitant to see __builtin_unreachable() getting used in release builds 
> in this particular scenario.  Having X crash just because we hit a 
> default switch case seems unnecessarily fragile.
>
Okay.  I'll send an updated version of the texture layers patch in a
minute taking into account your feed-back.

> My second concern is this: if someday someone really does want to use 
> __builtin_unreachable() in the context of unconventional flow control 
> (per gcc docs), will our new-and-improved unreachable() macro still be 
> the right tool for them?  I worry about changing the behavior of what 
> seems to be an established convention.
>
Yes, I'm pretty sure it would still be, after all the unreachable macro
doesn't have any effect other than eliminating compiler warnings [what
we would still get from the call to abort] and increasing optimization
opportunities [which are not a concern in debug builds].

> Sorry for dragging this out.  Please go ahead and post a patch showing 
> what you have in mind.
>
I already did [1].  Would you mind reviewing it?

[1] http://lists.freedesktop.org/archives/mesa-dev/2013-December/049902.html

> -Brian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131211/1a1445cc/attachment.pgp>