<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 29/06/15 13:26, Francisco Jerez
wrote:<br>
</div>
<blockquote cite="mid:87mvzi4s66.fsf@riseup.net" type="cite">
<pre wrap="">Davin McCall <a class="moz-txt-link-rfc2396E" href="mailto:davmac@davmac.org"><davmac@davmac.org></a> writes:
</pre>
<blockquote type="cite">
<pre wrap="">On 29/06/15 10:40, Francisco Jerez wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Davin McCall <a class="moz-txt-link-rfc2396E" href="mailto:davmac@davmac.org"><davmac@davmac.org></a> writes:
</pre>
<blockquote type="cite">
<pre wrap="">On 26/06/15 14:53, Francisco Jerez wrote:
</pre>
<blockquote type="cite">
<pre wrap="">[...]
Your first approach seemed quite reasonable IMHO. Were you able to
measure any performance regression from it?
Thanks.
</pre>
</blockquote>
<pre wrap="">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.
</pre>
</blockquote>
<pre wrap="">Most likely some assumption left to fix in the i965 back-end?
</pre>
</blockquote>
<pre wrap="">
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 :(
</pre>
<blockquote type="cite">
<pre wrap=""> Have you
shared your changes to the i965 driver already? They don't seem to be
part of your v1.
</pre>
</blockquote>
<pre wrap="">
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);
</pre>
</blockquote>
<pre wrap="">
This looks wrong, the previous code was taking a pointer to the tail
sentinel, but (exec_node *)&tail_sentinel.prev != &tail_sentinel.
</pre>
</blockquote>
<br>
You're right, thanks. However, when I fix this an assert triggers:<br>
<blockquote>glmark2: brw_fs_generator.cpp:2136: int
fs_generator::generate_code(const cfg_t*, int): Assertion
`!"Should be lowered by lower_load_payload()"' failed.<br>
</blockquote>
I'll have to keep investigating.<br>
<br>
Davin<br>
<br>
</body>
</html>