[Bug 34156] r600g performance regression between 780c183b and 862ebb41

Sat Feb 19 22:28:35 PST 2011

https://bugs.freedesktop.org/show_bug.cgi?id=34156

--- Comment #6 from Dave Witbrodt <dawitbro at sbcglobal.net> 2011-02-19 22:28:35 PST ---
I created a local branch using 077c448d as the start point, then reverted the 4
commits mentioned in Comment 5.  Performance was restored.

Since merging "master" onto this local branch resulted in loss of performance
again, I went looking for the cause.  Using 'git merge master' relocated the
revert commits to the top/newest part of the list, so I had to use 'git
rebase'.  After some experimentation, I was able to create a branch with the
following properties:

1. identical commits to "master" until 077c448d
2. local commits reverting 077c448d, 7687eaba, 1fa95c7f, and a77e813d
3. commits identical to those from "master" after 077c448d up through fd8d4b32.

With the commits applied in this order, I was able to investigate whether:

a. reverting Henri's 4 commits would restore performance [yes]
b. there was one or more commits added later which degraded performance [yes]

What follows is a sort of map of what I found using 'git bisect' after
reverting 077c448d, 7687eaba, 1fa95c7f, and a77e813d; I use ellipses to
indicate commits omitted, while commits not separated by an ellipsis appear
consecutively in 'git log':

[TESTED MANUALLY:  BAD]
commit fd8d4b32ede6ebeae332539b71d38c36420e2654
Author: Marek Olšák <maraeo at gmail.com>
Date:   Fri Feb 18 15:29:00 2011 +0100

    r300g: remove tracking whether vertex buffers need to be validated

...

[BAD]
commit 467023e8080489abeff53e18ac83560eaf851827
Author: Marek Olšák <maraeo at gmail.com>
Date:   Tue Feb 8 15:21:35 2011 +0100

    r600g: use the same upload buffer for vertices, indices, and constants

[GOOD]
commit 80424700574e128070f457d79e1920d512a1efda
Author: Thomas Hellstrom <thellstrom at vmware.com>
Date:   Tue Feb 8 01:14:22 2011 -0800

    mesa/st: Plug a fragment program variant parameter leak

...

[TESTED MANUALLY:  GOOD]
commit bb1036aae5b31af583b9aa263fec73c899383493
Author: Thomas Hellstrom <thellstrom at vmware.com>
Date:   Mon Feb 7 15:37:05 2011 +0100

    mesa/st: Fix vertex buffer leak

[IGNORED; IRRELEVANT ON MY SYSTEM]
commit 1e1b89103e1cdce9f20b9f32825dd166f8d9d6e1
Author: Kristian Høgsberg <krh at bitplanet.net>
Date:   Mon Feb 7 20:49:58 2011 -0500

    wayland-egl: Add struct wl_egl_display argument to +wl_egl_window_create()

[4 LOCAL COMMITS HERE REVERTING 077c448d, 7687eaba, 1fa95c7f, & a77e813d]

commit 077c448d184799e0d9ec962013ec784c6a5c1807
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:08 2011 +0100

    r600g: Add support for relative addressing on constant buffers.

commit 871460eb149b9868e5750f13b8206e271743c4a2
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:08 2011 +0100

    r600g: Set the fetch type in r600_bc_vtx_build().

commit 4c30a80e384d523803d70ead3403cf3ca30b8868
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Handle the ADD_INT instruction in r600_bc_get_num_operands().

commit 5c59eebfae55240a2308c02b0a6ad971c9b83304
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Generalize the pipe_add_vertex_attrib() functions.

commit b9fd1a1e4b2121225195056ea1b679d62c399ddb
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Remove vs_resource and ps_resource from the pipe context.

commit 7687eabaa0470261e059a2d6502628fffd209345
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Split constants in r600_shader_from_tgsi().

commit 1fa95c7f9e7f1b63364b1f9c6289690418cf6313
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Do the tgsi_full_src_register to r600_shader_src conversion in
    r600_shader_from_tgsi().

commit a77e813de32643ae2dfffd7ad12abed596172cab
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Split r600_bc_alu_src.

[GOOD]
commit 3b1c1f02537544a11772b94a8f2e8c3d4c886ca8
Author: Henri Verbeet <hverbeet at gmail.com>
Date:   Mon Feb 7 15:22:07 2011 +0100

    r600g: Store literal values in the r600_bc_alu_src structure.

...

As the bisect made clear, there was a second regression present after the one I
was trying to remove for this bug report; that regression occurs in this
commit:

    commit 467023e8080489abeff53e18ac83560eaf851827
    Author: Marek Olšák <maraeo at gmail.com>
    Date:   Tue Feb 8 15:21:35 2011 +0100

        r600g: use the same upload buffer for vertices, indices, and constants

I will open a new bug report for this.  I can confirm that reverting the 4
commits by Henri and this other commit by Marek restores performance. 
Currently, the HEAD of my Mesa repository is at fd8d4b32 (Feb. 18), and
performance is as good or better than it was before with just these 5 commits
reverted.

Let me make clear that I do not understand the code, and I am not sure whether
all 4 of Henri's commits cause a loss in performance.  I do know for certain
that a77e813d is the first to introduce a negative effect; I had to revert the
other three because they depended on the changes in that particular commit.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.