On 29/06/15 13:26, Francisco Jerez wrote:
Davin McCall <dav...@davmac.org> writes:
On 29/06/15 10:40, Francisco Jerez wrote:
Davin McCall <dav...@davmac.org> writes:
On 26/06/15 14:53, Francisco Jerez wrote:
[...]
Your first approach seemed quite reasonable IMHO. Were you able to
measure any performance regression from it?
Thanks.
When I run an apitrace replay of a Dota 2 trace [1] with
LIBGL_ALWAYS_SOFTWARE and without the patch I get (averaged over 5 runs):
Maximum Resident Set Size (kbytes): 4509696
FPS: .9044752
user time: 2467.306
("Maximum Resident Set Size" and user time are given by GNU "time". I'm
not sure what it's really measuring, because this is a 32-bit system and
I don't see how the maximum resident set could be > 4GB; "top" shows
virt+res capping out at about 2.3GB. However I assume MRSS is at least
giving some relative indication of memory use; the deviation wasn't too
high).
With the patch (again averaged over 5 runs):
Maximum Resident Set Size: 4523622.4
FPS: 0.9068524
user time: 2457.506
So, "MRSS" has gone up a bit, but nothing else has changed
significantly. I think that means memory use has slightly increased, but
performance hasn't really changed.
I wanted to test with the Intel driver using INTEL_NO_HW, but I get a
segfault when the patch is applied. Having checked over the patch
several times, I think this might mean that it triggers a latent bug
elsewhere, but I am still investigating that. V2 of the patch does not
trigger this crash.
Most likely some assumption left to fix in the i965 back-end?
That's what I thought. However, I've just tried (after reverting the
patch) inserting a single field in the exec_list structure to emulate
the data layout that should occur when the patch is applied - but I
can't then reproduce the problem. So I guess there is something wrong
with the patch itself, but I'm blind to it :(
Have you
shared your changes to the i965 driver already? They don't seem to be
part of your v1.
No, I've not shared them previously, but I've included them now (below).
Davin
[...]
diff --git a/src/mesa/drivers/dri/i965/brw_fs_builder.h
b/src/mesa/drivers/dri/i965/brw_fs_builder.h
index 58ac598..eeabd08 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_builder.h
+++ b/src/mesa/drivers/dri/i965/brw_fs_builder.h
@@ -85,7 +85,8 @@ namespace brw {
fs_builder
at_end() const
{
- return at(NULL, (exec_node *)&shader->instructions.tail);
+ return at(NULL,
+ (exec_node *)&shader->instructions.tail_sentinel.prev);
This looks wrong, the previous code was taking a pointer to the tail
sentinel, but (exec_node *)&tail_sentinel.prev != &tail_sentinel.
You're right, thanks. However, when I fix this an assert triggers:
glmark2: brw_fs_generator.cpp:2136: int
fs_generator::generate_code(const cfg_t*, int): Assertion `!"Should
be lowered by lower_load_payload()"' failed.
I'll have to keep investigating.
Davin
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
