On Sep 19, 2015 3:31 PM, "Jason Ekstrand" <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > On Sep 18, 2015 1:29 PM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: > > > > On Friday, September 18, 2015 10:55:34 AM Jason Ekstrand wrote: > > > Um, why? I'm sure you have some reason but I don't immediately see how a > > > progress flag from lower_vec_to_movs is useful. Some of the others > > > definitely are. > > > --Jason > > > > Sorry, I should have explained in at least one of the patches. > > > > I'm hoping to create an OPT() macro, similar to what we do for i965 > > passes. It would: > > > > 1. Run the pass. > > 2. Call nir_validate_shader() if we actually made any progress. > > 3. Optionally nir_print_shader() if we made any changes. > > (i.e. let us do INTEL_DEBUG=optimizer but for NIR passes) > > > > I need to know whether a pass made progress in order to decide whether > > or not to dump the shader. I might want to see the code before and > > after nir_lower_vec_to_movs(). > > > > Also, having some passes return 'bool' and some return 'void' is > > awkward: I can't wrap void-return functions in the same sort of macro. > > That makes sense. I think what I'd like to have is a data structure > > nir_shader_pass { > bool (*shader_func)(nir_shader *shader); > bool (*impl_func)(nir_function_impl *impl); > nir_metadata preserved; > }; > > Then have one of these exported per pass instead of the actual function or functions. Then we could have a helper function that runs the pass similar to what the OPT macro would. It could even be smart and, if you don't provide a shader_func, it runs impl_func on each overload. Does that seem reasonable? > --Jason I forgot to say that this is a good first pass and the series is Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>>