Bug 54832

Summary:	Piglit Regression: r300g swtcl 185ed21 draw: simplify index buffer specification
Product:	Mesa	Reporter:	Tom Stellard <tstellar>
Component:	Drivers/Gallium/r300	Assignee:	Default DRI bug account <dri-devel>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	CC:	brianp
Version:	git
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Backtrace from primitive-restart-DISABLE_VBO piglit test

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.