| Summary: | [llvmpipe] piglit ext_transform_feedback-immediate-reuse-uniform-buffer regression | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Vinson Lee <vlee> |
| Component: | Drivers/Gallium/llvmpipe | Assignee: | mesa-dev |
| Status: | RESOLVED MOVED | QA Contact: | mesa-dev |
| Severity: | normal | ||
| Priority: | medium | CC: | jfonseca, sroland |
| Version: | 10.6 | Keywords: | bisected, regression |
| Hardware: | x86-64 (AMD64) | ||
| OS: | Linux (All) | ||
| See Also: | https://bugs.freedesktop.org/show_bug.cgi?id=103762 | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
Ah yes I actually knew about that but didn't consider it all that important. The reason is that the code was already buggy and this change just exposed the bug. In short here's what happens: - the transform feedback buffer is only 4 bytes, which is later bound as constant buffer - constant buffers are expected to be in multiples of 16 bytes (as the contents are vec4 elements), thus the size is considered to be zero by (llvm) draw (and would be the same for llvmpipe fragment shaders as well). Before this change, this worked because we do not verify that an access to a buffer constant is within range of the buffer, IFF the access is done via an immediate (so, indirect access would always have returned zero even with the old code). This is in fact not very safe (I've got some quickly hacked up piglit test which tries to cause a crash but didn't manage really more than valgrind warnings). This did not change (though I think it probably will in the future), however for simplified shader code we now bind (1-element sized) fake buffers explicitly if the buffer size is zero (this was actually trivial to crash before just don't bind a constant buffer then any attempt at constant buffer lookup would have produced a nice clean null pointer dereference). I don't have time to fix these issues though for now. Well the one not verifying accesses outside of range ought to be easy but doesn't help this case, but the bug causing one I'm actually not quite sure HOW it should be fixed. Since it is not really ok to bind 4-byte sized buffers as a constant buffer which thus could be considered a state tracker bug but I'm not sure if it actually could do much about it. Of course could be easily hacked up in draw so the test passes again (could only use fake buffer if size is 0 before the divide by 16 for instance) but it wouldn't really serve much purpose, I'd rather have it fail so the issue doesn't get lost completely. -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/234. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
mesa: 1eac3ae1a6ebecf353054d937dd603a11ea33fb3 (master 10.6.0-devel) $ ./bin/ext_transform_feedback-immediate-reuse-uniform-buffer -auto Probe color at (16,0) Expected: 0.062500 0.937500 0.062500 Observed: 0.000000 0.000000 0.000000 Probe color at (32,0) Expected: 0.125000 0.875000 0.125000 Observed: 0.000000 0.000000 0.000000 Probe color at (48,0) Expected: 0.187500 0.812500 0.187500 Observed: 0.000000 0.000000 0.000000 Probe color at (64,0) Expected: 0.250000 0.750000 0.250000 Observed: 0.000000 0.000000 0.000000 Probe color at (80,0) Expected: 0.312500 0.687500 0.312500 Observed: 0.000000 0.000000 0.000000 Probe color at (96,0) Expected: 0.375000 0.625000 0.375000 Observed: 0.000000 0.000000 0.000000 Probe color at (112,0) Expected: 0.437500 0.562500 0.437500 Observed: 0.000000 0.000000 0.000000 Probe color at (128,0) Expected: 0.500000 0.500000 0.500000 Observed: 0.000000 0.000000 0.000000 Probe color at (144,0) Expected: 0.562500 0.437500 0.562500 Observed: 0.000000 0.000000 0.000000 Probe color at (160,0) Expected: 0.625000 0.375000 0.625000 Observed: 0.000000 0.000000 0.000000 Probe color at (176,0) Expected: 0.687500 0.312500 0.687500 Observed: 0.000000 0.000000 0.000000 Probe color at (192,0) Expected: 0.750000 0.250000 0.750000 Observed: 0.000000 0.000000 0.000000 Probe color at (208,0) Expected: 0.812500 0.187500 0.812500 Observed: 0.000000 0.000000 0.000000 Probe color at (224,0) Expected: 0.875000 0.125000 0.875000 Observed: 0.000000 0.000000 0.000000 Probe color at (240,0) Expected: 0.937500 0.062500 0.937500 Observed: 0.000000 0.000000 0.000000 PIGLIT: {"result": "fail" } 586536a4e1c34725b3b38c3425db569fac0c91e9 is the first bad commit commit 586536a4e1c34725b3b38c3425db569fac0c91e9 Author: Roland Scheidegger <sroland@vmware.com> Date: Thu Apr 9 00:49:11 2015 +0200 gallivm: don't use control flow when doing indirect constant buffer lookups llvm goes crazy when doing that, using way more memory and time, though there's probably more to it - this points to a very much similar issue as fixed in 8a9f5ecdb116d0449d63f7b94efbfa8b205d826f. In any case I've seen a quite plain looking vertex shader with just ~50 simple tgsi instructions (but with a dozen or so such indirect constant buffer lookups) go from a terribly high ~440ms compile time (consuming 25MB of memory in the process) down to a still awful ~230ms and 13MB with this fix (with llvm 3.3), so there's still obvious improvements possible (but I have no clue why it's so slow...). The resulting shader is most likely also faster (certainly seemed so though I don't have any hard numbers as it may have been influenced by compile times) since generally fetching constants outside the buffer range is most likely an app error (that is we expect all indices to be valid). It is possible this fixes some mysterious vertex shader slowdowns we've seen ever since we are conforming to newer apis at least partially (the main draw loop also has similar looking conditionals which we probably could do without - if not for the fetch at least for the additional elts condition.) v2: use static vars for the fake bufs, minor code cleanups Reviewed-by: Jose Fonseca <jfonseca@vmware.com> :040000 040000 3d9c36f85b3f7cf81413a06ea0454a4c035ea447 9d2ff2be20f10d6f129bc3e1a9979d2a97a37a79 M src bisect run success