search

parallaxinc / OpenSpin

Spin/PASM compiler for the Parallax Propeller.
56 stars 19 forks source link

Memory access alignment errors. #1

Closed ZiCog closed 10 years ago

ZiCog commented 10 years ago

There are a number of memory alignment errors. Mostly caused by casting from char pointers into ints or shorts whilst accessing various data structures.

Whist this is not an issue on most modern architectures it does cause a large slowdown in openspin when compiled to JavaScript and run in a browser environment.

The following is a grep of all the offending misaligned accesses found so far. They are marked with "FIXME Alignment error." Line numbers are only approximate as they changed a bit when instrumenting the code.

DistillObjects.cpp:212: memmove(&g_pCompilerData->dis[disPtr], &g_pCompilerData->dis[disPtr + (3 + numSubObjects)], (g_pCompilerData->dis_ptr - disPtr) * 2); // FIXME: Aligment error.

PropellerCompiler.cpp:911: short vsize = ((short)(&pData[0])); // FIXME: Alignment Error.

PropellerCompiler.cpp:912: short psize = ((short)(&pData[2])); // FIXME: Alignment error.

PropellerCompiler.cpp:962: int value = ((int)(&pData[1])); // FIXME: Alignment error.

PropellerCompiler.cpp:1412: unsigned char* pObj = &(g_pCompilerData->obj_data[g_pCompilerData->obj_offsets[i]]); // FIXME Alignment error.

PropellerCompiler.cpp:1418: objvar[i] = (int)(((unsigned short)(pObj))); // FIXME: Alignment error.

PropellerCompiler.cpp:1426: unsigned short psize = ((unsigned short)(pObj)); // FIXME: Alignment error.

StringConstantRoutines.cpp:102: ((short)&(g_pCompilerData->obj[g_pCompilerData->str_patch[strIndex]])) = strAddress; // FIXME: Aligment error.

Utilities.cpp:264: ((int)pSymbol) = temp; // FIXME Alignment error.

reltham commented 10 years ago

Check the latest commit (on Mar. 5th, number 6). I addressed all but the DistillObjects.cpp:212 & PropellerCompiler.cpp:1412 ones, which already appear to be legit. Not sure how to change them to be aligned? The DistillObjects one is likely just because it's a memmove, and your detection mechanism will hit on it. I also am now initializing all the statics/globals in main() that were previously not being initialized there.

ZiCog commented 10 years ago

I tested the alignment fixes (commit 1cb20dc40...) against my instrumentation on x86 and in the browser from an Emscripten compilation. Everything works so far.

For the Emscripten compile I was able to remove the compilation options for UNALIGNED_MEMORY which means faster code. Running with CHECK_HEAP_ALIGN no longer produces exceptions as it did before.

All in all looking good.

reltham commented 10 years ago

I'm closing this one now, since you reported it was good now.