lucas95123 / bullet

Automatically exported from code.google.com/p/bullet
Other
0 stars 0 forks source link

btSolverBody Allocator should alloc to 64 bytes not 16 #659

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Hi

we've been getting QNaNs randomly appearing in the solver code on the Vita due 
the fact that the compiler is making special optimisations based on the fact 
btSolverBody is 64 byte aligned. 

Unfortunately BT_DECLARE_ALIGNED_ALLOCATOR is only alloc-ing to 16-byte 
alignment.

I guess there should be a BT_DECLARE_ALIGNED_ALLOCATOR64 as well?

thanks
Matt

Original issue reported on code.google.com by matt8000...@googlemail.com on 16 Oct 2012 at 4:56

GoogleCodeExporter commented 8 years ago
I did a quick search and ATTRIBUTE_ALIGNED64 is only used in two places. 

In fact, the variables in btRigidBody that are 64byte aligned should be removed 
(m_deltaLinearVelocity, 
m_deltaAngularVelocity,m_invMass,m_pushVelocity,m_turnVelocity), because we 
re-introduced the btSolverBody.

Are the crashes fixed when replacing ATTRIBUTE_ALIGNED64 by ATTRIBUTE_ALIGNED16?

Can you test/profile if it is really worth adding 
ATTRIBUTE_ALIGNED64/BT_DECLARE_ALIGNED_ALLOCATOR64?

Thanks!

Original comment by erwin.coumans on 16 Oct 2012 at 5:04

GoogleCodeExporter commented 8 years ago
Hi Erwin,

After posting I noticed that btAlignedAllocator only does 16 byte aligned 
allocations so you'd have to pass through an argument to that as well!  
Probably much nicer to replace ATTRIBUTE_ALIGNED64 with ATTRIBUTE_ALIGNED16 as 
you suggest if it's not needed for performance reasons.  

I'll have a quick go at profiling the Vita code later today and see if there's 
any noticeable improvement.  I guess there should be some difference if 
compiler is optimising the code differently but I suspect it's small.

thanks
Matt

Original comment by matt8000...@googlemail.com on 17 Oct 2012 at 8:46

GoogleCodeExporter commented 8 years ago
Hi Matt,

Does changing to ATTRIBUTE_ALIGNED16 fix the issue for you?

Original comment by erwin.coumans on 7 Dec 2012 at 7:09

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r2633.

Original comment by erwin.coumans on 17 Dec 2012 at 9:29