dlls/d3dx9_36/bytecodewriter.c oddity

[Gerald Pfeifer]

Metteo et al,

I noticed d3dx9_36/bytecodewriter.c can be simplified as follows, but
somewhat tells me this may rather point out a problem somewhere in that
code, so I am not sending this to wine-patches.

(The only functional difference should be the missing WARN in some cases,
and I could restore that if desired, but...see above.)

Thoughts?  Guidance?

Gerald

diff --git a/dlls/d3dx9_36/bytecodewriter.c 
b/dlls/d3dx9_36/bytecodewriter.c
index 07f96e7..e7bd8a5 100644
--- a/dlls/d3dx9_36/bytecodewriter.c
+++ b/dlls/d3dx9_36/bytecodewriter.c
@@ -1035,7 +1035,7 @@ static const struct bytecode_backend vs_1_x_backend = {
 static void instr_ps_1_0123_texld(struct bc_writer *This,
                                   const struct instruction *instr,
                                   struct bytecode_buffer *buffer) {
-    DWORD idx, srcidx;
+    DWORD idx;
     struct shader_reg reg;
     DWORD swizzlemask;

@@ -1074,17 +1074,6 @@ static void instr_ps_1_0123_texld(struct bc_writer *This,
         /* map the temp dstreg to the ps_1_3 texture temporary register */
         This->funcs->dstreg(This, &instr->dst, buffer, instr->shift, instr->dstmod);
     } else if(instr->src[0].type == BWRITERSPR_TEMP) {
-        if(instr->src[0].regnum == T0_REG) {
-            srcidx = 0;
-        } else if(instr->src[0].regnum == T1_REG) {
-            srcidx = 1;
-        } else if(instr->src[0].regnum == T2_REG) {
-            srcidx = 2;
-        } else if(instr->src[0].regnum == T3_REG) {
-            srcidx = 3;
-        } else {
-            WARN("Invalid address data source register r%u\n", instr->src[0].regnum);
-        }

swizzlemask = (3 << BWRITERVS_SWIZZLE_SHIFT) |
             (3 << (BWRITERVS_SWIZZLE_SHIFT + 2)) |

[Matteo Bruni]

2010/8/18 Gerald Pfeifer :
> Metteo et al,
>
> I noticed d3dx9_36/bytecodewriter.c can be simplified as follows, but
> somewhat tells me this may rather point out a problem somewhere in that
> code, so I am not sending this to wine-patches.
>
> (The only functional difference should be the missing WARN in some cases,
> and I could restore that if desired, but...see above.)
>
> Thoughts?  Guidance?
>
> Gerald
>
> diff --git a/dlls/d3dx9_36/bytecodewriter.c
> b/dlls/d3dx9_36/bytecodewriter.c
> index 07f96e7..e7bd8a5 100644
> --- a/dlls/d3dx9_36/bytecodewriter.c
> +++ b/dlls/d3dx9_36/bytecodewriter.c
> @@ -1035,7 +1035,7 @@ static const struct bytecode_backend vs_1_x_backend = {
>  static void instr_ps_1_0123_texld(struct bc_writer *This,
>                                   const struct instruction *instr,
>                                   struct bytecode_buffer *buffer) {
> - DWORD idx, srcidx;
> - DWORD idx;
>
> struct shader_reg reg;
>     DWORD swizzlemask;
>
> @@ -1074,17 +1074,6 @@ static void instr_ps_1_0123_texld(struct bc_writer *This,
>         /* map the temp dstreg to the ps_1_3 texture temporary register */
>         This->funcs->dstreg(This, &instr->dst, buffer, instr->shift, instr->dstmod);
>     } else if(instr->src[0].type == BWRITERSPR_TEMP) {
> - if(instr->src[0].regnum == T0_REG) {
> - srcidx = 0;
> - } else if(instr->src[0].regnum == T1_REG) {
> - srcidx = 1;
> - } else if(instr->src[0].regnum == T2_REG) {
> - srcidx = 2;
> - } else if(instr->src[0].regnum == T3_REG) {
> - srcidx = 3;
> - } else {
> - WARN("Invalid address data source register r%u\n", instr->src[0].regnum);
> - }
>
> swizzlemask = (3 << BWRITERVS_SWIZZLE_SHIFT) |
>             (3 << (BWRITERVS_SWIZZLE_SHIFT + 2)) |

Hmm, so srcidx is unused. Yes, that piece of code is useless now, it's
a remnant of an older version of that function where the source
register was handled by some ad-hoc code, which I since then replaced
with a call to the generic This->funcs->srcreg(). The function is
correct otherwise, there are some tests to confirm that.
In my opinion you can send the patch as is. Alternatively you can
replace that piece of code with just a check for instr->src[0].regnum
!= T[0123]_REG, to keep the WARN, but it's not worth the effort
probably.
Thanks!

[Gerald Pfeifer]

On Thu, 19 Aug 2010, Matteo Bruni wrote:
> Hmm, so srcidx is unused. Yes, that piece of code is useless now, it's
> a remnant of an older version of that function where the source
> register was handled by some ad-hoc code, which I since then replaced
> with a call to the generic This->funcs->srcreg(). The function is
> correct otherwise, there are some tests to confirm that.
> In my opinion you can send the patch as is. Alternatively you can
> replace that piece of code with just a check for instr->src[0].regnum
> != T[0123]_REG, to keep the WARN, but it's not worth the effort
> probably.

Thanks a lot Matteo for your guidance!  Based on this, I'll formally
submit this to wine-patches.

Gerald