Series is Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
On 12/18/2017 03:26 PM, Francisco Jerez wrote: > The weight_vector_type constructor was inadvertently assuming C++17 > semantics of the new operator applied on a type with alignment > requirement greater than the largest fundamental alignment. > Unfortunately on earlier C++ dialects the implementation was allowed > to raise an allocation failure when the alignment requirement of the > allocated type was unsupported, in an implementation-defined fashion. > It's expected that a C++ implementation recent enough to implement > P0035R4 would have honored allocation requests for such over-aligned > types even if the C++17 dialect wasn't active, which is likely the > reason why this problem wasn't caught by our CI system. > > A more elegant fix would involve wrapping the __SSE2__ block in a > '__cpp_aligned_new >= 201606' preprocessor conditional and continue > taking advantage of the language feature, but that would yield lower > compile-time performance on old compilers not implementing it > (e.g. GCC versions older than 7.0). > > Fixes: af2c320190f3c731 "intel/fs: Implement GRF bank conflict mitigation > pass." > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104226 > Reported-by: Józef Kucia <joseph.ku...@gmail.com> > --- > src/intel/compiler/brw_fs_bank_conflicts.cpp | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/src/intel/compiler/brw_fs_bank_conflicts.cpp > b/src/intel/compiler/brw_fs_bank_conflicts.cpp > index 0cd880d44f2..e87fcbfc5eb 100644 > --- a/src/intel/compiler/brw_fs_bank_conflicts.cpp > +++ b/src/intel/compiler/brw_fs_bank_conflicts.cpp > @@ -277,13 +277,10 @@ namespace { > struct weight_vector_type { > weight_vector_type() : v(NULL), size(0) {} > > - weight_vector_type(unsigned n) : > - v(new vector_type[DIV_ROUND_UP(n, vector_width)]()), > - size(n) {} > + weight_vector_type(unsigned n) : v(alloc(n)), size(n) {} > > weight_vector_type(const weight_vector_type &u) : > - v(new vector_type[DIV_ROUND_UP(u.size, vector_width)]()), > - size(u.size) > + v(alloc(u.size)), size(u.size) > { > memcpy(v, u.v, > DIV_ROUND_UP(u.size, vector_width) * sizeof(vector_type)); > @@ -291,7 +288,7 @@ namespace { > > ~weight_vector_type() > { > - delete[] v; > + free(v); > } > > weight_vector_type & > @@ -304,6 +301,19 @@ namespace { > > vector_type *v; > unsigned size; > + > + private: > + static vector_type * > + alloc(unsigned n) > + { > + const unsigned align = MAX2(sizeof(void *), > __alignof__(vector_type)); > + const unsigned size = DIV_ROUND_UP(n, vector_width) * > sizeof(vector_type); > + void *p; > + if (posix_memalign(&p, align, size)) > + return NULL; > + memset(p, 0, size); > + return reinterpret_cast<vector_type *>(p); > + } > }; > > /** > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
