Porting MASM5 code in Quake 2 to GAS -- Unexpected rendering results

[maraakate]

Hello, not sure if this should be here or in the game development area (apologies if I posted in the wrong place)

I am porting Quake 2's inline assembly code for MSVC to MASM then finally to GAS (for use with MinGW).  The specific code in question is for the Skin drawing (R_PolysetCalcGradients for those who want to look it up).  The code almost "works" what happens is the skin seems to stretched over the model incorrectly.



A few interesting things I noticed is when I run objdump -dwrC r_polysa.obj > r_polysa.masmand the same for the GAS version the code is mostly similar except for the fact fsubp and fsubrp have been swapped in MASM.  Please note not the operands (I already know about this issue in GAS).

In the picture the left side is the GAS version, the right side is the MASM version.  The original MASM code (and therefore what I have in GAS) is what should be on the left side.  I am unsure why MASM is apparently swapping this or if objdump is reporting it incorrectly.  However, if I swap the two this does not fix the issue.  It just gets inverted in another wrong way.


I mention this FSUBP/FSUBRP swap because this was a problem in porting the Particle blending inline ASM code.  This had one call to use an FSUBRP in the MASM version, objdump reported it now being FSUBP and I had to change it to FSUBP in the GAS version for it to work!  I don't understand why this is happening?

In any case, I am new to assembly, but understand some basics and have been doing some reading.  Obviously the math here is not quite right, but it seems as though it should be.  I don't know how or what to do next.  How do I fix and debug this problem?

The code repository to what I am working on is at: https://bitbucket.org/neozeed/q2dos/commits/branch/win32_asm  (specifically the Win32_ASM branch).  The files I am working with are gas\r_polysa.s and ref_soft\r_polysa.asm.

[Siekmanski]

I'm not sure but, it looks like the vertices are drawn in the wrong order.
Instead of drawing the triangles in ABC order you could try to draw them in ACB order.

[jj2007]

A few interesting things I noticed is when I run objdump -dwrC r_polysa.obj > r_polysa.masmand the same for the GAS version the code is mostly similar except for the fact fsubp and fsubrp have been swapped in MASM.  Please note not the operands (I already know about this issue in GAS).

In the picture the left side is the GAS version, the right side is the MASM version.  The original MASM code (and therefore what I have in GAS) is what should be on the left side.  I am unsure why MASM is apparently swapping this or if objdump is reporting it incorrectly.  However, if I swap the two this does not fix the issue.  It just gets inverted in another wrong way.

Your left side looks wrong. Here's a quick test:

include \masm32\MasmBasic\MasmBasic.inc         ; download
  Init
  FpuFill  ; loads the FPU with values 1001 ... 1008
  deb 4, "Start", ST(0), ST(1), ST(2), ST(3)
  fsubp st(3), st
  deb 4, "Result fsubp st(3), st", ST(0), ST(1), ST(2), ST(3)
  fsubrp st(1), st
  deb 4, "Result fsubrp st(1), st", ST(0), ST(1), ST(2), ST(3)
  Inkey
EndOfCode

Under the hood, Olly reports this:

Code Select Expand


DEEB                        fsubp st(3), st                          ; float 1001.0000000000000000
DEE1                        fsubrp st(1), st

And the result looks like this:

Code Select Expand

Start
ST(0)           1001.000000000000000
ST(1)           1002.000000000000000
ST(2)           1003.000000000000000
ST(3)           1004.000000000000000

Result fsubp st(3), st
ST(0)           1002.000000000000000
ST(1)           1003.000000000000000
ST(2)           3.000000000000000000
ST(3)           1005.000000000000000

Result fsubrp st(1), st
ST(0)           -1.000000000000000000
ST(1)           3.000000000000000000
ST(2)           1005.000000000000000
ST(3)           1006.000000000000000

From the manuals:
DE E8+i   FSUBP ST(i), ST(0)   Subtract ST(0) from ST(i), store result in ST(i), and pop register stack
DE E0+i   FSUBRP ST(i), ST(0)   Subtract ST(i) from ST(0), store result in ST(i), and pop register stack

[maraakate]

I'm not sure but, it looks like the vertices are drawn in the wrong order.
Instead of drawing the triangles in ABC order you could try to draw them in ACB order.

I am using the original code that was in the MASM, nothing has been changed except swapping src/dst for GAS (and same for any fp arithmetic because of a quirk in AT&T syntax things like fsubp st(1), st(0) are reversed in GAS i.e. fsubp %st(0), %st(1)).

[jj2007]

As written above, your left side looks wrong: This is not the correct encoding, it should be de eb and de e1

[maraakate]

<snip>

I have some ignorance on this as assembly is new to me.  Are you saying that GAS is generating the wrong opcodes? 

I have been procrastinating learning it for years and since I work on this Quake 2 engine project and this was on our "TODO" forever I figured this would be a good way to learn it as I go.  And so far this has taught me things in regards to assembly.

So what I have done so far is: compile the game in Visual Studio with MASM, load the game, attach a debugger, set a breakpoint at the start of this problem function, run the first demo.  From here I have been stepping into line-by-line to watch the values being modified especially the first 3 FP registers as the code is using these.  I got about two "loops" in with writing it down and following it.  From there I then recompile the game with MinGW (which uses GAS), load the game, attach a debugger, etc...  and so far the values are identical in the FP registers.

I will post the entire code from both sides.

[maraakate]

As written above, your left side looks wrong: This is not the correct encoding, it should be de eb and de e1

Most of the file looks like this when dumped via objdump with GCC.  I am not sure if objdump doesn't "do" well on VS obj files, or if there are compiler settings in MASM vs GAS that is causing things to be generated differently.  By encoding, you mean the opcodes?

[jj2007]

Are you saying that GAS is generating the wrong opcodes?

See reply #4. It should be de eb and de e1, no idea which of your tools generates the wrong ones...

[maraakate]

Problem function that I converted is R_PolysetCalcGradients.  R_PolysetDrawSpans8_Opaque was always in MASM and GAS files and has never been touched, and works fine.  However R_PolysetCalcGradients does call it.

Here is the original MASM code.  Compiler flags used:  ml /c /Cp /coff /Fo$(OUTDIR)\$(InputName).obj /Zm /Zi $(InputPath)
https://pastebin.com/vN7HumZ1

Here is the converted GAS code compiler flags: -m32 -Wall -g -fno-strict-aliasing
https://pastebin.com/J29NkR2W

Please note in GAS I swapped the fsubp and fsubrp's because that was what it said in the objdump compares.  So far at about the second iteration in the numbers are the same values in the FP registers.

[maraakate]

So swapping the fsubp and fsubrp has fixed those opcodes.  Again, I noticed this in my port of the particles blending that an fsubrp in MASM had to become fsubp when I studied the objdump.  Still unsure as to why.  However, the second photo in the first post is what happens when I swap fsubp and fsubrp so it's still not right.  However, I see there are other opcodes that aren't the same either but I'm assuming some of that is just where it is accessing the memory since the layout may be slightly different?  Should I be worried?

[jj2007]

Please note in GAS I swapped the fsubp and fsubrp's

You should swap the operands, the syntax requires that of course, but why the instructions? That doesn't make sense.

[maraakate]

If I swap the operands it generates wrong code again with dbe3 and dbe9

Code Select Expand


gas/r_polysa.s: Assembler messages:
gas/r_polysa.s:135: Warning: translating to `fsubp %st(0),%st(3)'
gas/r_polysa.s:136: Warning: translating to `fsubrp %st(0),%st(1)'


This is an actual problem with AT&T syntax.  See: https://en.wikibooks.org/wiki/X86_Assembly/GAS_Syntax#Caveats

The UnixWare assembler, and probably other AT&T derived ix86 Unix assemblers, generate floating point instructions with reversed source and destination registers in certain cases. Unfortunately, gcc and possibly many other programs use this reversed syntax, so we're stuck with it.

For example

        fsub %st,%st(3)

results in %st(3) being updated to %st - %st(3) rather than the expected %st(3) - %st. This happens with all the non-commutative arithmetic floating point operations with two register operands where the source register is %st and the destination register is %st(i).

Note that even objdump -d -M intel still uses reversed opcodes, so use a different disassembler to check this. See http://bugs.debian.org/372528 for more info.

So not sure if objdump is even generating the right things.  Would it be worth posting the MASM obj file here and my GAS obj file so we can see?

[jj2007]

You could post an exe where an int 3 is inserted as a breakpoint before the fsubxx.

[maraakate]

You will need Quake 2, should be able to download the demo for this.  If not I can supply something to test with:

http://maraakate.org/q2/Q2DEMO.7z -> The official q2 demo.  Includes MASM/VS version below with int 3 breakpoint. (Approx 30MB)

http://maraakate.org/q2/q2_masm.7z -> MASM/VS version

http://maraakate.org/q2/q2_mingw_with_fsubp_first.7z -> MinGW/GAS version where fsubp is first, then fsubrp.

Code Select Expand


fsubp %st(0), %st(3)     // t0 * p10_minus_p20 | t1 * p00_minus_p20 | t1 * p01_minus_p21 - t0 * p11_minus_p21
fsubrp %st(0), %st(1)     // t1 * p00_minus_p20 - t0 * p10_minus_p20 | t1 * p01_minus_p21 - t0 * p11_minus_p21


http://maraakate.org/q2/q2_mingw_with_fsubrp_swap.7z -> MinGW/GAS version where fsubrp comes BEFORE the fsubp.

Code Select Expand


fsubrp %st(0), %st(3)      // t0 * p10_minus_p20 | t1 * p00_minus_p20 | t1 * p01_minus_p21 - t0 * p11_minus_p21
fsubp %st(0), %st(1)      // t1 * p00_minus_p20 - t0 * p10_minus_p20 | t1 * p01_minus_p21 - t0 * p11_minus_p21


All versions are compiled with debug information.  You could attach a source file to it in visual studio at least for the MASM version.  MinGW would require installation of VisualGDB for that.  Or you can get familiar with using GDB :S.

If you're unable to attach to process because it loads the demo and quits right away you can start the game with q2.exe +disconnect when you've attached simply run demomap q2demo1.dm2 to start the demo.

[hutch--]

It may be easier if your GAS version supports it to use Intel rather than AT&T notation.