On 04/16/2018 10:55 PM, Thomas Helland wrote:
> Hi, and thanks for the patch =)
>
> Have you done any performance testing on this to verify it
> gives us a speedup of any kind? I'm asking because it seems like
> this might be something that a decent compiler should be able to do.
> Performance related patches, at least in core mesa, usually have
> some justification with benchmark numbers in the commit message.
> Some style comments below
>
> 2018-04-17 1:03 GMT+02:00 Vlad Golovkin <vlad.golovkin.m...@gmail.com>:
>> When GLmatrix elements and its inverse are stored contiguously in memory it
>> is possible to
>> allocate, free and copy these fields with 1 function call instead of 2.
>> ---
>> src/mesa/math/m_matrix.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
>> index 57a49533de..4ab78a1fb3 100644
>> --- a/src/mesa/math/m_matrix.c
>> +++ b/src/mesa/math/m_matrix.c
>> @@ -1438,8 +1438,7 @@ _math_matrix_is_dirty( const GLmatrix *m )
>> void
>> _math_matrix_copy( GLmatrix *to, const GLmatrix *from )
>> {
>> - memcpy(to->m, from->m, 16 * sizeof(GLfloat));
>> - memcpy(to->inv, from->inv, 16 * sizeof(GLfloat));
>> + memcpy(to->m, from->m, 16 * 2 * sizeof(GLfloat));
>> to->flags = from->flags;
>> to->type = from->type;
>> }
>> @@ -1470,12 +1469,17 @@ _math_matrix_loadf( GLmatrix *mat, const GLfloat *m )
>> void
>> _math_matrix_ctr( GLmatrix *m )
>> {
>> - m->m = _mesa_align_malloc( 16 * sizeof(GLfloat), 16 );
>> + m->m = _mesa_align_malloc( 16 * 2 * sizeof(GLfloat), 16 );
>> if (m->m)
>> + {
>
> Our style guides says to keep the curly bracket after an if on the same line.
>
>> + m->inv = m->m + 16;
>> memcpy( m->m, Identity, sizeof(Identity) );
>> - m->inv = _mesa_align_malloc( 16 * sizeof(GLfloat), 16 );
>> - if (m->inv)
>> memcpy( m->inv, Identity, sizeof(Identity) );
>> + }
>> + else
>> + {
>
> } else {
>
> Although I see that this file defaults to;
>
> {
> else {
>
> for some reason. Feel free to follow existing style, or adjust to my comments.
I think that's because this file is really, really old, and it pre-dates
the current style.
> Also, if we want to do this change it deserves a comment in the source.
>> + m->inv = NULL;
>> + }
>> m->type = MATRIX_IDENTITY;
>> m->flags = 0;
>> }
>> @@ -1493,7 +1497,6 @@ _math_matrix_dtr( GLmatrix *m )
>> _mesa_align_free( m->m );
>> m->m = NULL;
>>
>> - _mesa_align_free( m->inv );
>> m->inv = NULL;
>> }
>>
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
