[Mesa-dev] [PATCH 1/2] intel/fs/bank_conflicts: Use posix_memalign() instead of overaligned new to obtain vector storage.

[Francisco Jerez]

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.kucia at 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);
+      }
    };
 
    /**
-- 
2.14.2