Page MenuHome

BGE: Fix: 16 byte heap alignment compiler warning (level 3 C4316) for Bullet classes
AcceptedPublic

Authored by Thomas Szepe (hg1) on May 3 2015, 8:49 AM.

Details

Summary

If we not ensure 16 byte alignment it is possible that bad things could happen, when instructions expecting 16 byte alignment operate on the data (SSE instruction operating on misaligned data).

Diff Detail

Event Timeline

Thomas Szepe (hg1) retitled this revision from to BGE: Fix: 16 byte heap alignment compiler warning (level 3 C4316) for Bullet classes.
Thomas Szepe (hg1) updated this object.
Thomas Szepe (hg1) updated this object.

Actually I am unsure about this patch, because I found this solution in the internet.
For Windows it seems to work, but I don't know if it cause a problem on Linux or Mac.
I have added Daniel Stokes, because I think he has a Mac.

I think is Dalai who has the Mac computer.

Porteries Tristan (panzergame) edited edge metadata.

Looks good to me :)

This revision is now accepted and ready to land.May 3 2015, 11:18 AM

I'm still unclear on what this patch is really trying to solve. At the moment I don't believe the BGE makes much use of SSE, so I don't really know how much we need to worry about alignment. Is there some documentation from Bullet saying we should do this? I'd favor readability and simplicity over performance in BGE code at this point in time. Or is there some reason other than performance to do this?

I do not have a Mac, just a Windows and Linux install. Are these changes sufficient, or do we need to do more to be properly aligned for Bullet? Also, what impact are you seeing with these changes?

Actually it seems to work everything without his patch. But I want to get rid of the 16 compiler warnings.Which are caused by this.
Also I always want to do the save way.

Is there some documentation from Bullet saying we should do this?

I don't know I nerve reeded the whole Bullet doku, But the compiler will do and while searching in the internet to find a fix for this warnings, I found some user's the had problems (crashes) without it. But I saw that Bullet it also use it internally.
Maybe in future if we use Eigen (SMID using SSE) this can cause an error without it.

Are these changes sufficient, or do we need to do more to be properly aligned for Bullet? Also, what impact are you seeing with these changes?

Actually the compiler don't give me more warnings. So I think it is OK.
As I know. without it the compiler use two 8 byte alignment to store the 16 byte data on the heap. So it can happen that the two data blocks are stored on different paces on the heap.
If then an SSE command want grab an 16 byte block, this will cause mostly a crash.

I have a Mac but no time for testing on my end (still tons of multiview issues, sorry). You can use the experimental-build branch with builder.blender.org to test OSX building.

From a quick glance this patch shouldn't be necessary since the affected classes aren't used by Bullet.

So you only need to align memory that's used to store e.g. a btRigidBody.

As for OSX, it shouldn't cause any problems but again it shouldn't be necessary.

Maybe I overlooked something, but imo if there is a problem with alignment it's probably somewhere else.