Bugzilla – Bug 8283
i915 can't swizzle TEX arguments
Last modified: 2009-08-24 12:24:28 UTC
i915 doesn't support swizzling TEX arguments. The attached patch solves that by
first creating a table of what R regs are "taken" for each instruction. Then,
when a TEX with swizzle arg is encountered, this information is used to pick a
free R reg, which will take the swizzled argument, so TEX can be invoked w/o
Created attachment 6989 [details] [review]
Add swizzle support for TEX FP op.
Firstly, the approach looks like a good one, but a couple of comments:
- Please use universal diffs (diff -u) as they're easier for others to read...
- It looks like you don't seem to cope with the situation where
get_free_rreg() fails, ie. the return value isn't checked.
- There should already be a 'NumInstructions' field hanging around, so you
shouldn't need to scan the list twice.
- It would be good to move the calculation of temporaries used into core
mesa somewhere, perhaps taking the regsNeeded array as an argument. But I don't
think this is a big deal at this point.
Otherwise, it looks good - nice work.
Actually I think the calculation of neededRegs is incorrect, consider:
MOV B, 99
MOV C, B
TEX X, Y.zyx
MOV B, B
MOV A, B
Working backwards, I think that after MOV B, B the fact that B is still live
will be lost, so regsNeeded at the TEX instruction will be incorrect and B could
be used as a temporary.
Created attachment 6993 [details] [review]
Add swizzle support for TEX FP op. (try 2)
Incorporated feedback from comments. The case in comment #3 should now be
handled properly, I think.
Created attachment 7095 [details] [review]
TEXLD swizzle nr 3
Does this version work for you? I've slightly cleaned things up:
- move calculate into its own function
- use the existing mesa call for the number of argments of an instruction
- just pass the bitmask to emit_texld so that code doesn't learn too much
about mesa's fragment program representation.
The bug priority was upgraded (P2->high) with the bugzilla configuration change.
I'm Changing the priority back to the normal one.
Sorry for the spam.
any response, Frank?
*** Bug 10786 has been marked as a duplicate of this bug. ***
The latest (In reply to comment #5)
> Created an attachment (id=7095) [details]
> TEXLD swizzle nr 3
This patch works fine for me and I check it into git (3369cd9a6f943365242d7832e69788d4aede9a8f)
*** Bug 14122 has been marked as a duplicate of this bug. ***
Haihao, the fix is not at mesa_7_0_branch. Please pick it up. Thanks!
(In reply to comment #12)
> Haihao, the fix is not at mesa_7_0_branch. Please pick it up. Thanks!
Still doesn't work for me (Using the 7.0.3 mesa driver version bundled with OpenSuSE 11.0)
Also I can't seem to be able to use your "patches" (although I'm about 98% sure I'm doing something wrong - file saved as "patch.cgi", chmoded to 764) Here is the error I'm getting:
./patch.cgi: line 1: ?: command not found
./patch.cgi: line 2: ?: command not found
./patch.cgi: line 3: ?: command not found
./patch.cgi: line 4: ?: command not found
./patch.cgi: line 5: Index:: command not found
./patch.cgi: line 6: ===================================================================: command not found
./patch.cgi: line 7: RCS: command not found
./patch.cgi: line 8: retrieving: command not found
diff: invalid option -- '.'
diff: Try `diff --help' for more information.
./patch.cgi: line 10: ---: command not found
./patch.cgi: line 11: +++: command not found
./patch.cgi: line 12: @@: command not found
./patch.cgi: line 16: +#include: command not found
./patch.cgi: line 20: @@: command not found
./patch.cgi: line 24: syntax error near unexpected token `('
./patch.cgi: line 24: `+#define I915_MAX_INSN (I915_MAX_TEX_INSN+I915_MAX_ALU_INSN)'
Could someone tell me what I'm doing wrong / suggest another fix / do I have have to file a bug report for the patch (don't think so, though: seems to be my fault...)
This patch has been pushed into mesa_7_0_branch.
Mass version move, cvs -> git