| Summary: | bug in r300 compiler | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Tomasz P. <son_of_the_osiris> |
| Component: | Drivers/Gallium/r300 | Assignee: | David Heidelberg (okias) <david> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | david, tstellar |
| Version: | git | ||
| Hardware: | All | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
s/signed/int/
0001-r300g-compiler-Fix-unsigned-comparison-with-less-tha.patch |
||
Created attachment 80074 [details] [review] s/signed/int/ Comment on attachment 80074 [details] [review] s/signed/int/ 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. https://bugs.freedesktop.org/show_bug.cgi?id=54867 So, still not pushed in today git, can someone push this small fix? (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. Created attachment 87247 [details] [review] 0001-r300g-compiler-Fix-unsigned-comparison-with-less-tha.patch Hi Tom, sending correctly formated commit message. David Ping? |
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.
Orginally posted on mailing list: 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