Bug 54867 - bug in r300 compiler
bug in r300 compiler
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r300
All Linux (All)
: medium normal
Assigned To: David Heidelberg (okias)
Depends on:
  Show dependency treegraph
Reported: 2012-09-13 14:19 UTC by Tomasz P.
Modified: 2013-10-25 14:03 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:

s/signed/int/ (1.04 KB, patch)
2013-05-31 02:22 UTC, Chistopher Krakowiak
Details | Splinter Review
0001-r300g-compiler-Fix-unsigned-comparison-with-less-tha.patch (1.12 KB, patch)
2013-10-07 16:09 UTC, David Heidelberg (okias)
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomasz P. 2012-09-13 14:19:44 UTC
Orginally posted on mailing list:

In playing with Coccinelle, I discovered a signed/unsigned bug in

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

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

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

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 1 Chistopher Krakowiak 2013-05-31 02:22:18 UTC
Created attachment 80074 [details] [review]
Comment 2 Tom Stellard 2013-06-03 13:31:28 UTC
Comment on attachment 80074 [details] [review]

Review of attachment 80074 [details] [review]:

This patch looks good to me, but for the commit message, you need to wrap long lines to 80 or fewer characters (I actually wrap to 75, but I'm not sure what the standard convention is) and re-organize the commit message in the form of:

Code area: Brief description

Long description (if necessary)

Link to fixed bugs

For this patch, it should be something like:

r300g/compiler: Fix unsigned comparison with less than zero

rc_find_free_temporary_list() returns signed integer (in case of lack of free temporary registersreturns -1), so new_index in radeon_rename_regs() should be signed.

Comment 3 David Heidelberg (okias) 2013-10-04 20:02:15 UTC
So, still not pushed in today git, can someone push this small fix?
Comment 4 Tom Stellard 2013-10-07 13:41:57 UTC
(In reply to comment #3)
> So, still not pushed in today git, can someone push this small fix?

I can push it if you provide an updated patch with a proper commit message.
Comment 5 David Heidelberg (okias) 2013-10-07 16:09:51 UTC
Created attachment 87247 [details] [review]

Hi Tom,

sending correctly formated commit message.

Comment 6 Fabio Pedretti 2013-10-15 08:41:40 UTC