On 08/27/2016 11:18 AM, Jan Vesely wrote:
On Fri, 2016-08-26 at 17:25 +0100, Eric Engestrom wrote:
On Fri, Aug 26, 2016 at 04:21:14PM +0200, Kai Wasserbäch wrote:
Hey Eric,
Eric Engestrom wrote on 26.08.2016 15:49:
On Fri, Aug 26, 2016 at 03:14:57PM +0200, Kai Wasserbäch wrote:
Brian Paul wrote on 26.08.2016 14:50:
On 08/26/2016 05:58 AM, Kai Wasserbäch wrote:
Cc: Brian Paul <bri...@vmware.com>
Signed-off-by: Kai Wasserbäch <k...@dev.carbon-project.org>
---
Hi Brian,
is this what you had in mind? If so, I was wondering
whether virgl_encode.c
would need to be updated as well. Doesn't seem like it,
since the functions
there map everything to uint32_t or some other standard
type.
Another point are the switch statements nouveau uses. To
silence the -Wswitch
warning of GCC I stuck a default case with two asserts at
the end of them. But
maybe it would be better to use an if...else for nv30 and
nv50.
I think one assertion is enough. It's up to the nouveau
developers whether they
want to do more.
ok, then I'll go for the generic "unhandled type" assert.
I would've gone with `unreachable()`, since nothing outside the
enum range should ever get in.
Do you feel strongly about that and require a v3?
I don't, it's fine as is.
Personally I think the
assert() is better since the function could be called with another
enum member
value, which still is unhandled by the switch().
unreachable() still has an assert() (see src/util/macros.h:75)
The difference is that unreachable() tells the compiler that we won't
be
interested in what comes next. If we reach it, we abort with a
message
in debug builds, and after that the compiler can do whatever it
wants,
e.g. optimise out irrelevant code paths, and obviously it wont warn
about these code paths anymore, which is often more interesting.
moreover, assert leaves untested codepath with silent failure in
ndebug builds.
In this case it would be better to use unreachable + unhandled enum
values instead of default (so the compiler complains when new shader
types get added, however unlikely that is).
just my 2c,
Jan
Kai, the series LGTM. I'll add my R-b and push the four patches.
Do you plan to do more such as pipe_context::set_constant_buffer()
Also, git grep 'unsigned shader' and 'uint shader' shows quite a few
more places where similar clean-ups are possible. I'll probably take
care of the VMware driver.
As for assert vs. unreachable, whichever people think is better is fine
with me. Off-hand, I'm not aware of any new types of shaders coming
along any time soon so I think it's a minor issue which can be addressed
in follow-on commits.
Thanks for doing this!
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
