On Thursday, September 22, 2016 1:54:44 PM PDT Ian Romanick wrote: > On 09/21/2016 10:20 PM, Kenneth Graunke wrote: > > In the past, we imported the prototypes of built-in functions, generated > > calls to those, and waited until link time to resolve the calls and > > import the actual code for the built-in functions. > > I thought part of the reason we did this was to account for some of the > weird desktop GLSL rules about overriding and overloading built-in > functions. Does this still handle that nonsense correctly? I remember > you spent a lot of time rewritting this code to get all that right.
Yeah, that all works fine. I thought it'd be a problem at first too. The rules about function matching remain the same. So, we'll find the same function we found before. Before this patch, we'd import a prototype and tag it as "this is a built-in" or not. The linker would use that to link against the built-in or the user function. Now, instead of emitting a tagged-prototype, we generate the built-in inline. That means that all prototypes are user functions. The linker doesn't need to think about built-ins anymore. Works out surprisingly well. Passes all the tests. > There's one additional question below. > > > This severely limited our compile-time optimization opportunities: even > > trivial functions like dot() were represented as function calls. We > > also had no way of reasoning about those calls; they could have been > > 1,000 line functions with side-effects for all we knew. > > > > Practically all built-in functions are trivial translations to > > ir_expression opcodes, so it makes sense to just generate those inline. > > Since we eventually inline all functions anyway, we may as well just do > > it for all built-in functions. > > > > There's only one snag: built-in functions that refer to built-in global > > variables need those remapped to the variables in the shader being > > compiled, rather than the ones in the built-in shader. Currently, > > ftransform() is the only function matching those criteria, so it seemed > > easier to just make it a special case. > > > > On Skylake: > > > > total instructions in shared programs: 12023491 -> 12024010 (0.00%) > > instructions in affected programs: 77595 -> 78114 (0.67%) > > helped: 97 > > HURT: 309 > > > > total cycles in shared programs: 137239044 -> 137295498 (0.04%) > > cycles in affected programs: 16714026 -> 16770480 (0.34%) > > helped: 4663 > > HURT: 4923 > > > > while these statistics are in the wrong direction, the number of > > hurt programs is small (309 / 41282 = 0.75%), and I don't think > > anything can be done about it. A change like this significantly > > alters the order in which optimizations are performed. > > > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > > --- > > src/compiler/glsl/ast_function.cpp | 46 > > ++++++++++++++++++-------------------- > > 1 file changed, 22 insertions(+), 24 deletions(-) > > > > diff --git a/src/compiler/glsl/ast_function.cpp > > b/src/compiler/glsl/ast_function.cpp > > index 7e62ab7..ac3b52d 100644 > > --- a/src/compiler/glsl/ast_function.cpp > > +++ b/src/compiler/glsl/ast_function.cpp > > @@ -430,7 +430,8 @@ generate_call(exec_list *instructions, > > ir_function_signature *sig, > > exec_list *actual_parameters, > > ir_variable *sub_var, > > ir_rvalue *array_idx, > > - struct _mesa_glsl_parse_state *state) > > + struct _mesa_glsl_parse_state *state, > > + bool inline_immediately) > > The caller just passes sig->is_builtin() for this parameter. We already > pass sig into this function. Do we need this new parameter? I suppose not. I thought "inline immediately" might be a reasonable option for generate_call(), in case we wanted to use it in other cases. Then again, I can't think of another case where we'd want it. I'm happy to drop it. What do you prefer?
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
