This sounds good to me, but I guess it is not really fixing anything, right? I ask because the subject claims that this patch does something that the original code was already supposed to be doing.
On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote: > Before, we bailing in assign_constant_locations based on the minimum > dispatch size. The more direct thing to do is simply to check for > whether or not we have constant locations and bail if we do. For > nir_setup_uniforms, it's completely safe to do it multiple times > because > we just copy a value from the NIR shader. > --- > src/intel/compiler/brw_fs.cpp | 4 +++- > src/intel/compiler/brw_fs_nir.cpp | 5 ++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/intel/compiler/brw_fs.cpp > b/src/intel/compiler/brw_fs.cpp > index 52079d3..75139fd 100644 > --- a/src/intel/compiler/brw_fs.cpp > +++ b/src/intel/compiler/brw_fs.cpp > @@ -1956,8 +1956,10 @@ void > fs_visitor::assign_constant_locations() > { > /* Only the first compile gets to decide on locations. */ > - if (dispatch_width != min_dispatch_width) > + if (push_constant_loc) { > + assert(pull_constant_loc); > return; > + } > > bool is_live[uniforms]; > memset(is_live, 0, sizeof(is_live)); > diff --git a/src/intel/compiler/brw_fs_nir.cpp > b/src/intel/compiler/brw_fs_nir.cpp > index 7556576..05efee3 100644 > --- a/src/intel/compiler/brw_fs_nir.cpp > +++ b/src/intel/compiler/brw_fs_nir.cpp > @@ -81,8 +81,11 @@ fs_visitor::nir_setup_outputs() > void > fs_visitor::nir_setup_uniforms() > { > - if (dispatch_width != min_dispatch_width) > + /* Only the first compile gets to set up uniforms. */ > + if (push_constant_loc) { > + assert(pull_constant_loc); > return; > + } > > uniforms = nir->num_uniforms / 4; > } _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev