[Mesa-dev] [PATCH 1/3] st/mesa: use MIN2 macro in favour of custom _min function

Ian Romanick idr at freedesktop.org
Fri Feb 21 16:34:11 PST 2014


On 02/21/2014 03:15 PM, Marek Olšák wrote:
> The main reason _min exists is that it evaluates each parameter once,
> while MIN2 evaluates each parameter twice, so in this case, MIN2 calls
> the get_param function twice, which may not be desirable. The same for
> the other macros.

There is a GCC extension that cope with this, but I don't know if other 
compilers support it.  It has existed in GCC for a decade or more. 
Directly from the GCC documentation:

      #define max(a,b) \
        ({ typeof (a) _a = (a); \
            typeof (b) _b = (b); \
          _a > _b ? _a : _b; })

If some variation of this works with MSVC, maybe we could just "fix" our 
MIN2 and MAX2 macros.

Perhaps Vinson could research our options here...

If nothing else, this is the Nth time I've seen patches like this 
rejected.  Perhaps the _min and _maxf functions could get a comment 
explaining why they exist?

> Marek
>
> On Fri, Feb 21, 2014 at 11:59 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>   src/mesa/state_tracker/st_extensions.c | 27 +++++++++++----------------
>>   1 file changed, 11 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
>> index e43e7b4..a909b71 100644
>> --- a/src/mesa/state_tracker/st_extensions.c
>> +++ b/src/mesa/state_tracker/st_extensions.c
>> @@ -39,11 +39,6 @@
>>   #include "st_extensions.h"
>>   #include "st_format.h"
>>
>> -static unsigned _min(unsigned a, unsigned b)
>> -{
>> -   return (a < b) ? a : b;
>> -}
>> -
>>   static float _maxf(float a, float b)
>>   {
>>      return (a > b) ? a : b;
>> @@ -72,19 +67,19 @@ void st_init_limits(struct st_context *st)
>>      boolean can_ubo = TRUE;
>>
>>      c->MaxTextureLevels
>> -      = _min(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_2D_LEVELS),
>> +      = MIN2(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_2D_LEVELS),
>>               MAX_TEXTURE_LEVELS);
>>
>>      c->Max3DTextureLevels
>> -      = _min(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_3D_LEVELS),
>> +      = MIN2(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_3D_LEVELS),
>>               MAX_3D_TEXTURE_LEVELS);
>>
>>      c->MaxCubeTextureLevels
>> -      = _min(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS),
>> +      = MIN2(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS),
>>               MAX_CUBE_TEXTURE_LEVELS);
>>
>>      c->MaxTextureRectSize
>> -      = _min(1 << (c->MaxTextureLevels - 1), MAX_TEXTURE_RECT_SIZE);
>> +      = MIN2(1 << (c->MaxTextureLevels - 1), MAX_TEXTURE_RECT_SIZE);
>>
>>      c->MaxArrayTextureLayers
>>         = screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS);
>> @@ -168,7 +163,7 @@ void st_init_limits(struct st_context *st)
>>         }
>>
>>         pc->MaxTextureImageUnits =
>> -         _min(screen->get_shader_param(screen, sh,
>> +         MIN2(screen->get_shader_param(screen, sh,
>>                                          PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS),
>>                 MAX_TEXTURE_IMAGE_UNITS);
>>
>> @@ -185,7 +180,7 @@ void st_init_limits(struct st_context *st)
>>         pc->MaxTemps           = pc->MaxNativeTemps           =
>>            screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_TEMPS);
>>         pc->MaxAddressRegs     = pc->MaxNativeAddressRegs     =
>> -         _min(screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_ADDRS),
>> +         MIN2(screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_ADDRS),
>>                 MAX_PROGRAM_ADDRESS_REGS);
>>         pc->MaxParameters      = pc->MaxNativeParameters      =
>>            screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_CONSTS);
>> @@ -196,7 +191,7 @@ void st_init_limits(struct st_context *st)
>>            screen->get_shader_param(screen, sh, PIPE_SHADER_CAP_MAX_CONST_BUFFERS);
>>         if (pc->MaxUniformBlocks)
>>            pc->MaxUniformBlocks -= 1; /* The first one is for ordinary uniforms. */
>> -      pc->MaxUniformBlocks = _min(pc->MaxUniformBlocks, MAX_UNIFORM_BUFFERS);
>> +      pc->MaxUniformBlocks = MIN2(pc->MaxUniformBlocks, MAX_UNIFORM_BUFFERS);
>>
>>         pc->MaxCombinedUniformComponents = (pc->MaxUniformComponents +
>>                                             c->MaxUniformBlockSize / 4 *
>> @@ -240,16 +235,16 @@ void st_init_limits(struct st_context *st)
>>      }
>>
>>      c->MaxCombinedTextureImageUnits =
>> -         _min(c->Program[MESA_SHADER_VERTEX].MaxTextureImageUnits +
>> +         MIN2(c->Program[MESA_SHADER_VERTEX].MaxTextureImageUnits +
>>                 c->Program[MESA_SHADER_GEOMETRY].MaxTextureImageUnits +
>>                 c->Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits,
>>                 MAX_COMBINED_TEXTURE_IMAGE_UNITS);
>>
>>      /* This depends on program constants. */
>>      c->MaxTextureCoordUnits
>> -      = _min(c->Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits, MAX_TEXTURE_COORD_UNITS);
>> +      = MIN2(c->Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits, MAX_TEXTURE_COORD_UNITS);
>>
>> -   c->MaxTextureUnits = _min(c->Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits, c->MaxTextureCoordUnits);
>> +   c->MaxTextureUnits = MIN2(c->Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits, c->MaxTextureCoordUnits);
>>
>>      c->Program[MESA_SHADER_VERTEX].MaxAttribs = MIN2(c->Program[MESA_SHADER_VERTEX].MaxAttribs, 16);
>>
>> @@ -749,7 +744,7 @@ void st_init_extensions(struct st_context *st)
>>         ctx->Extensions.ARB_texture_buffer_object = GL_TRUE;
>>
>>         ctx->Const.MaxTextureBufferSize =
>> -         _min(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE),
>> +         MIN2(screen->get_param(screen, PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE),
>>                 (1u << 31) - 1);
>>         ctx->Const.TextureBufferOffsetAlignment =
>>            screen->get_param(screen, PIPE_CAP_TEXTURE_BUFFER_OFFSET_ALIGNMENT);
>> --
>> 1.9.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



More information about the mesa-dev mailing list