On 09/22/2016 06:54 PM, Kenneth Graunke wrote: > 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?
I was going to say I'd rather drop it, but I changed my mind. I think it's fine as is. This patch is Reviewed-by; Ian Romanick <ian.d.roman...@intel.com>
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
