Bug 54832 - Piglit Regression: r300g swtcl 185ed21 draw: simplify index buffer specification
Summary: Piglit Regression: r300g swtcl 185ed21 draw: simplify index buffer specification
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r300 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-13 02:27 UTC by Tom Stellard
Modified: 2012-09-22 14:34 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Backtrace from primitive-restart-DISABLE_VBO piglit test (3.94 KB, text/plain)
2012-09-17 00:46 UTC, Tom Stellard
Details

Description Tom Stellard 2012-09-13 02:27:19 UTC
Lightsmark stopped working on my r300 laptop with swtcl following this commit:

commit 185ed2105829d6f5eb19edb9abbf0d7977e157c3
Author: Brian Paul <brianp@vmware.com>
Date:   Fri May 25 09:44:53 2012 -0600

    draw: simplify index buffer specification
    
    Replace draw_set_index_buffer() and draw_set_mapped_index_buffer() with
    draw_set_indexes() which simply takes a pointer and an index size.


When I start Lightsmark, it crashes during the "loading..." screen after a few seconds, before the demo starts rendering the scene.  Prior to the crash, the debug statement ("warning index out of range") @ draw_pt_vsplit_tmp.h:59 is printed several times, followed by the message:

draw_pipeline_run: max_index (7589) outside vertex buffer (300)

and then a segfault.
Comment 1 Tomasz P. 2012-09-13 13:05:08 UTC
Someone on the mailing list suspects that in r300 compiler is somekind of bug. He wrote:

In playing with Coccinelle, I discovered a signed/unsigned bug in
radeon_rename_regs.c:rc_rename_regs.

unsigned new_index;
unsigned writemask;
struct rc_variable * var = var_ptr->Item;

if (var->Inst->U.I.DstReg.File != RC_FILE_TEMPORARY) {
        continue;
}

new_index = rc_find_free_temporary_list(c, used, used_length,
                                                RC_MASK_XYZW);
if (new_index < 0) {
        rc_error(c, "Ran out of temporary registers\n");
        return;
}

unsigned new_index is compared with < 0.

I don't know the code, but I can't imagine that you'd need an unsigned
to represent a register index value.

Matt Turner
Comment 2 Tom Stellard 2012-09-13 13:26:16 UTC
(In reply to comment #1)
> Someone on the mailing list suspects that in r300 compiler is somekind of bug.
> He wrote:
> 
> In playing with Coccinelle, I discovered a signed/unsigned bug in
> radeon_rename_regs.c:rc_rename_regs.
> 
> unsigned new_index;
> unsigned writemask;
> struct rc_variable * var = var_ptr->Item;
> 
> if (var->Inst->U.I.DstReg.File != RC_FILE_TEMPORARY) {
>         continue;
> }
> 
> new_index = rc_find_free_temporary_list(c, used, used_length,
>                                                 RC_MASK_XYZW);
> if (new_index < 0) {
>         rc_error(c, "Ran out of temporary registers\n");
>         return;
> }
> 
> unsigned new_index is compared with < 0.
> 
> I don't know the code, but I can't imagine that you'd need an unsigned
> to represent a register index value.
> 
> Matt Turner

Please file a separate bug for this.
Comment 3 Tomasz P. 2012-09-13 14:21:30 UTC
done, bug: 54867
Comment 4 Tom Stellard 2012-09-17 00:43:08 UTC
This commit also regresses the following piglit tests:

Segfault:

primitive-restart-DISABLE_VBO
primitive-restart-VBO_COMBINED_VERTEX_AND_INDEX
primitive-restart-VBO_INDEX_ONLY
primitive-restart-VBO_SEPARATE_VERTEX_AND_INDEX
primitive-restart-VBO_VERTEX_ONLY

Fail:

draw-batch

Note: This can be reproduced on any r300g supported hardware by setting the environment variable: RADEON_NO_TCL=1
Comment 5 Tom Stellard 2012-09-17 00:46:53 UTC
Created attachment 67261 [details]
Backtrace from primitive-restart-DISABLE_VBO piglit test
Comment 6 Marek Olšák 2012-09-17 01:37:25 UTC
This should fix this:

http://lists.freedesktop.org/archives/mesa-dev/2012-September/027435.html
Comment 7 Tom Stellard 2012-09-17 02:13:28 UTC
(In reply to comment #6)
> This should fix this:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2012-September/027435.html

It does fix it, thanks!
Comment 8 Marek Olšák 2012-09-22 14:34:07 UTC
I pushed the fix into master and 9.0. Closing.


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.