-march=i686 causing lightmap corruption?

[Spirit]

I have no idea about engines or C.

I had a weird bug in reQuiem recently, where apparently lightmaps were corrupted. Here is an example. Top is how it looked like, bottom is how it should look like.





Luckily I was able to bisect to the culprit: The compiler flag "-march=i686".

I am on a x86_64 Linux, nvidia binary drivers. If I recall correctly I added that flag to be able to compile a 32 bit binary. The engine is 32 bit (if you say it like that), at least I think it is.

How can this option cause this? Do I actually need it? Does it hint at some bug in the engine code?

I left it in the project for now, if you want to try it for yourself grab the source at https://github.com/SpiritQuaddicted/reQuiem and compile it with codeblocks.

[szo]

Does the following help?
http://sourceforge.net/p/quakespasm/patches/15/
http://sourceforge.net/p/quakespasm/code/908/

[Spirit]

Sadly not. But I found that there were similar blotches on mfxsp17 that this change fixed.

mfxsp17 before and after:



Some more screenshots of the issue, from jam1_scampie and jam1_ericw:

[Baker]

How can this option cause this? Do I actually need it? Does it hint at some bug in the engine code?

Compiler and compiler flag issues are scary. I hit a compiler bug in one of the very first times I ever was working on ProQuake.

http://quakeone.com/forums/quake-help/q ... #post29325 (I played it cool, but notice a 7 hour gap until my next post! What was I doing those 7 hours? Being frustrated!)

Megazoid couldn't shoot healthboxes on E1M1. I checked all my code, nothing should have caused the issue because I really hadn't changed that much and certainly nothing to do with collisions, etc.

[Although I didn't need his advice on this occasion, I was fortunately enough that Ben Jardrup was willing to help me when I dealt with issues beyond my understanding.]

[szo]

It definitely implies that there is a bug somewhere, be that a bug
in C code or a bad use of a compiler flag.

I tried building it, the makefile was broken, fixed it. (can send to
you if you want.) Then realized that there is a codeblocks thing,
sigh.. (Why do I need codeblocks when there can be a simple makefile..
sigh^2...)

There are some "interesting" compiler flags in there. Things I noted:
* __linux__ defined among the CPPFLAGS, which is brain damage: your
compiler defines it already if it is targetting linux.
* -funsigned-char: why is it needed really???
* -fexpensive-optimizations: unless you are using gcc < 2.7 (or was
it <= 2.7??), it is already in effect.
* -fno-strength-reduce: why is this really needed?
* -funroll-loops: is it really worth the trouble??
* -O6 (in the makefiles): gcc highest optim.level is 3, -O6 is stupid
* -ffast-math: be careful with it.. (see the gcc man page.)

gl_brush.c is still broken: s/"contents ount"/count/ around line 1395

I didn't run into the broken lightmaps, but I only ran stock id1 maps
and only briefly. If you point me to the maps in question, maybe I can
look into it sometime.

[Spirit]

I tried building it, the makefile was broken, fixed it. (can send to
you if you want.) Then realized that there is a codeblocks thing,
sigh.. (Why do I need codeblocks when there can be a simple makefile..
sigh^2...)

Working makefile would be great! Codeblocks is just needed because that is how I got the source.

There are some "interesting" compiler flags in there. Things I noted:
* __linux__ defined among the CPPFLAGS, which is brain damage: your
compiler defines it already if it is targetting linux.
* -funsigned-char: why is it needed really???
* -fexpensive-optimizations: unless you are using gcc < 2.7 (or was
it <= 2.7??), it is already in effect.
* -fno-strength-reduce: why is this really needed?
* -funroll-loops: is it really worth the trouble??
* -O6 (in the makefiles): gcc highest optim.level is 3, -O6 is stupid
* -ffast-math: be careful with it.. (see the gcc man page.)

I have no idea about this stuff, I am just maintaining the engine. See http://forums.inside3d.com/viewtopic.php?f=1&t=5269 for some back story. I have no idea about jdhack's background, maybe he was unexperienced as well.

gl_brush.c is still broken: s/"contents ount"/count/ around line 1395

EXPLETIVES! Fixed, thanks.

I didn't run into the broken lightmaps, but I only ran stock id1 maps
and only briefly. If you point me to the maps in question, maybe I can
look into it sometime.

jam1_otp from https://www.quaddicted.com/reviews/func_mapjam1.html shows it right when spawning.

[szo]

Sent a small patch to your private email address.

I experimented with the cpu flags and -ffast-math: -march=pentium
(i586) is ok, but anything higher than that (-march=pentiumpro,
i686, pentium2. pentium3) in combination with -ffast-math reveals
the glitch you see. Seems like some floating point inaccuracy
somewhere. Howerver, either removing -ffast-math or switching to
SSE floating point math (i.e. -msse -mfpmath=sse in combination
with a proper -march, like -march=pentium3) "cures" the issue.
Didn't try to dig more.

[ericw]

Interesting..

Sounds like you nailed it down szo; I imagine -ffast-math is messing up that sensitive dot product in a way that lowers the precision a bit.
http://stackoverflow.com/questions/7420 ... ctually-do

On a related topic, the 'light' tool needs the same patch (adding the casts to increase the intermediate precision of that dot product), if you want to compile 'light' with sse. I sent it to Tyrann who applied it to tyrutils, so tyrutils from git is now safe to use as a 64-bit build.

Btw, I casted to 'long double' in the tyrutils patch, which is at least the size of double; in practice it's 128-bit on my laptop. Since the original code was being built for x87 (80-bit), i figured it's a bit safer than casting to double. Not sure if it will ever matter though.

[Spirit]

Thanks for the patches, I commited them.

I tried "-march=pentium4" and that worked.

Do I need either option anyways if I want to just build some binary for everyone to use? Should I have separate 32 and 64 binaries? It's all way over my head, I just want to release a proper build some day.

[szo]

I tried "-march=pentium4" and that worked.
Do I need either option anyways if I want to just build some binary for everyone to use? Should I have separate 32 and 64 binaries? It's all way over my head, I just want to release a proper build some day.

If you want your binary to work for everyone, I suggest you either do not use any cpu options, or use march=pentium for x86 builds which is pretty much guaranteed to work for evenyone. As for 64 bits, if the engine is 64bit-clean and you can build it, then build it already

[Spike]

if its just because your program truncates pointer precision due to it using the wrong data types, you can use -m32 and that'll work on x86, arm, or powerpc chips.
you can specify -m64, but that's generally default anyway (unless cross-compiling), and thus redundant.

I don't know what the actual difference is between the builds, but lightmap sizes are definitely dependant upon float precision. which sucks. you could try using the floor() libc function instead of casting to int perhaps? or add a tiny smidge extra first.