Devilution: On ideas to fix GetDirection8

[Predelnik]

Original PR: #622
There are two problems with GetDirection8 one is that variables in second array (or several arrays) are intialized out of order they placed on stack (initialization of 3rd three and 4th three is reversed), the second problem is that if we suppose that there is 4 arrays as psx symbols say then they get aligned by 4 and unlike single array make function not bin-exact. But the initialization order problem is easily solved with 4 arrays. There's two possible solutions to this I came up with:

1. Manual initialization of two-dimensional array in required order:
   https://github.com/Predelnik/devilution/commit/d12a2eb4b125032291baa2aa25f2b01d0d0a7182

   • Pros:

     • GetDirection16 may technically have this sort of initialization too

   • Cons:

     • Does not match to PSX symbols.
     • Initialization looks too verbose

2. __declspec(align(1)) fix.
   https://github.com/Predelnik/devilution/commit/0318c5b633795c0daa8336bad2c9e831bf4b0d1b

   • Pros:

     • Matches PSX symbols
     • Concise initialization

   • Cons:

     • Non-portable
     • Adding declspec to similar arrays in GetDirection16 ruins its bin-exactness.

Maybe there's some ideas how to force dense alignment less hacky than __declspec(align(1))? Then I think second option would clearly win.

[AJenbo]

I vote 1

[mewmew]

I vote 1

I too prefer keeping it portable. Vote 1.

[qndel]

1

[ghost]

Nah, I vote 3. Find the correct way to fix it, no hacky workarounds.

[ghost]

Hmm, I tried fooling around with optimizations but nothing seems to force stack alignment. I'd say for now go with solution 2 and add an ifdef to the declspec so it's compatible with other compilers.
Edit: the compiler option Full Optimization makes the function smaller but still not right.

[Predelnik]

But the only difference of /OX from /O2 seems to be missing /GF, /GY, wonder if they affect this. Difference between /O1//O2 however is /Ot//Os switches, maybe that's more important.

[sskras]

Maybe this is offset sensitive stuff too, after all?

[mewmew]

Maybe this is offset sensitive stuff too, after all?

These are local variables so the alignment would be on the function stack frame, not the global address space. As such, it seem less likely that this would be affected by other functions than itself.

[sskras]

Ah, right. I wonder whether #pragma pack(1) could be used at some point in missiles.cpp .

[Predelnik]

Here's slightly prettified version with declspec (align (1)) with ifdef which uses only single declspec
https://github.com/Predelnik/devilution/commit/0318c5b633795c0daa8336bad2c9e831bf4b0d1b

I think we may still keep this open for a bit in case if better solution will be found.