[Mesa-dev] [PATCH 7/7] mesa: add KHR_no_error support for glUseProgram
Nicolai Hähnle
nhaehnle at gmail.com
Sun May 7 20:29:53 UTC 2017
On 04.05.2017 16:28, Nicolai Hähnle wrote:
> On 04.05.2017 05:34, Timothy Arceri wrote:
>>
>>
>> On 04/05/17 13:31, Dave Airlie wrote:
>>>> +/* The ARB_separate_shader_object spec says:
>>>> + *
>>>> + * "The executable code for an individual shader stage is taken
>>>> from
>>>> + * the current program for that stage. If there is a current
>>>> program
>>>> + * object established by UseProgram, that program is considered
>>>> current
>>>> + * for all stages. Otherwise, if there is a bound program
>>>> pipeline
>>>> + * object (section 2.14.PPO), the program bound to the appropriate
>>>> + * stage of the pipeline object is considered current."
>>>> + */
>>>> +#define
>>>> USE_PROGRAM(no_error) \
>>>> + if (program)
>>>> { \
>>>> + /* Attach shader state to the binding point
>>>> */ \
>>>> + _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
>>>> &ctx->Shader); \
>>>> + /* Update the program
>>>> */ \
>>>> + _mesa_use_shader_program(ctx,
>>>> shProg); \
>>>> + } else
>>>> { \
>>>> + /* Must be done first: detach the progam
>>>> */ \
>>>> + _mesa_use_shader_program(ctx,
>>>> shProg); \
>>>> + /* Unattach shader_state binding point
>>>> */ \
>>>> + _mesa_reference_pipeline_object(ctx,
>>>> &ctx->_Shader, \
>>>> +
>>>> ctx->Pipeline.Default); \
>>>> + /* If a pipeline was bound, rebind it
>>>> */ \
>>>> + if (ctx->Pipeline.Current)
>>>> { \
>>>> +
>>>> _mesa_BindProgramPipeline##no_error(ctx->Pipeline.Current->Name); \
>>>> +
>>>> }
>>>> \
>>>> +
>>>> }
>>>> \
>>>> +
>>>
>>> why the macro, inline functions are a thing, or just one common
>>> function that both entrypoints call.
>>>
>>
>> So that we can avoid adding an if to call
>>
>> _mesa_BindProgramPipeline_no_error vs _mesa_BindProgramPipeline
>
> I have to say I also don't like this very much. Do you have benchmarks
> to back up the usefulness of this?
>
> This is where C++ templates would be great, because there could just be
> a boolean no_error template parameter. Maybe there's a way to get
> somewhere similar in C, even if it requires some compiler extensions?
I thought about this some more, and I feel that at least with gcc/clang,
the always_inline attributes provides a reasonably elegant solution. I
mean this code pattern:
__attribute((always_inline))
static inline void useprogram(GLuint program, bool check_error)
{
GET_CURRENT_CTX(ctx);
...
}
void GLAPIENTRY
_mesa_UseProgram(GLuint program)
{
useprogram(program, true);
}
void GLAPIENTRY
_mesa_UseProgram_no_error(GLuint program)
{
useprogram(program, false);
}
This would avoid the code duplication and macro magic that I'm finding a
bit worrisome while still getting fully optimized code in both paths.
This could also be used to clean up some of the other _no_error variants.
There's a bit of a question of other compiler support. At least judging
from the online docs, MSVC's __forceinline should have the same effect,
so it looks like all the relevant bases are covered.
What do you think?
Cheers,
Nicolai
>
> Cheers,
> Nicolai
>
>
>>
>>> Dave.
>>>
>>>> +void GLAPIENTRY
>>>> +_mesa_UseProgram_no_error(GLuint program)
>>>> +{
>>>> + GET_CURRENT_CONTEXT(ctx);
>>>> + struct gl_shader_program *shProg = NULL;
>>>> +
>>>> + if (program) {
>>>> + shProg = _mesa_lookup_shader_program(ctx, program);
>>>> + }
>>>> +
>>>> + USE_PROGRAM(_no_error)
>>>> +}
>>>> +
>>>>
>>>> void GLAPIENTRY
>>>> _mesa_UseProgram(GLuint program)
>>>> {
>>>> GET_CURRENT_CONTEXT(ctx);
>>>> struct gl_shader_program *shProg = NULL;
>>>>
>>>> if (MESA_VERBOSE & VERBOSE_API)
>>>> _mesa_debug(ctx, "glUseProgram %u\n", program);
>>>>
>>>> @@ -1875,44 +1915,21 @@ _mesa_UseProgram(GLuint program)
>>>> return;
>>>> }
>>>>
>>>> #ifdef DEBUG
>>>> if (ctx->_Shader->Flags & GLSL_USE_PROG) {
>>>> print_shader_info(shProg);
>>>> }
>>>> #endif
>>>> }
>>>>
>>>> - /* The ARB_separate_shader_object spec says:
>>>> - *
>>>> - * "The executable code for an individual shader stage is
>>>> taken from
>>>> - * the current program for that stage. If there is a current
>>>> program
>>>> - * object established by UseProgram, that program is
>>>> considered current
>>>> - * for all stages. Otherwise, if there is a bound program
>>>> pipeline
>>>> - * object (section 2.14.PPO), the program bound to the
>>>> appropriate
>>>> - * stage of the pipeline object is considered current."
>>>> - */
>>>> - if (program) {
>>>> - /* Attach shader state to the binding point */
>>>> - _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
>>>> &ctx->Shader);
>>>> - /* Update the program */
>>>> - _mesa_use_shader_program(ctx, shProg);
>>>> - } else {
>>>> - /* Must be done first: detach the progam */
>>>> - _mesa_use_shader_program(ctx, shProg);
>>>> - /* Unattach shader_state binding point */
>>>> - _mesa_reference_pipeline_object(ctx, &ctx->_Shader,
>>>> ctx->Pipeline.Default);
>>>> - /* If a pipeline was bound, rebind it */
>>>> - if (ctx->Pipeline.Current) {
>>>> - _mesa_BindProgramPipeline(ctx->Pipeline.Current->Name);
>>>> - }
>>>> - }
>>>> + USE_PROGRAM()
>>>> }
>>>>
>>>>
>>>> void GLAPIENTRY
>>>> _mesa_ValidateProgram(GLuint program)
>>>> {
>>>> GET_CURRENT_CONTEXT(ctx);
>>>> validate_program(ctx, program);
>>>> }
>>>>
>>>> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
>>>> index 99b4fe8..0a28185 100644
>>>> --- a/src/mesa/main/shaderapi.h
>>>> +++ b/src/mesa/main/shaderapi.h
>>>> @@ -120,20 +120,22 @@ _mesa_IsProgram(GLuint name);
>>>>
>>>> extern GLboolean GLAPIENTRY
>>>> _mesa_IsShader(GLuint name);
>>>>
>>>> extern void GLAPIENTRY
>>>> _mesa_LinkProgram(GLuint programObj);
>>>>
>>>> extern void GLAPIENTRY
>>>> _mesa_ShaderSource(GLuint, GLsizei, const GLchar* const *, const
>>>> GLint *);
>>>>
>>>> +void GLAPIENTRY
>>>> +_mesa_UseProgram_no_error(GLuint);
>>>> extern void GLAPIENTRY
>>>> _mesa_UseProgram(GLuint);
>>>>
>>>> extern void GLAPIENTRY
>>>> _mesa_ValidateProgram(GLuint);
>>>>
>>>>
>>>> extern void GLAPIENTRY
>>>> _mesa_BindAttribLocation(GLuint program, GLuint, const GLchar *);
>>>>
>>>> --
>>>> 2.9.3
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list