Improve MTdataHolder design and use it in math_brute_force (#1490)

Improve the design of the MTdataHolder wrapper:

 * Make it a class instead of a struct with a private member, to make
   it clearer that there is no direct access to the MTdata member.

 * Make the 1-arg constructor `explicit` to avoid unintended
   conversions.

 * Forbid copy construction/assignment as MTdataHolder is never
   initialised from an MTdataHolder object in the codebase.

 * Define move construction/assignment as per the "rule of five".

Use the MTdataHolder class throughout math_brute_force, to simplify
code by avoiding manual resource management.

Original patch by Marco Antognini.

Signed-off-by: Marco Antognini <marco.antognini@arm.com>
Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>

Signed-off-by: Marco Antognini <marco.antognini@arm.com>
Signed-off-by: Sven van Haastregt <sven.vanhaastregt@arm.com>

test_common/harness/mt19937.h

@@ -94,23 +94,42 @@ double genrand_res53(MTdata /*data*/);
 bool genrand_bool(MTdata /*data*/);
 
 #include <cassert>
+#include <utility>
 
-struct MTdataHolder
-{
-    MTdataHolder(cl_uint seed)
+class MTdataHolder {
+public:
+    MTdataHolder() = default;
+    explicit MTdataHolder(cl_uint seed)
     {
         m_mtdata = init_genrand(seed);
         assert(m_mtdata != nullptr);
     }
 
-    MTdataHolder(MTdata mtdata): m_mtdata(mtdata) {}
+    // Forbid copy.
+    MTdataHolder(const MTdataHolder&) = delete;
+    MTdataHolder& operator=(const MTdataHolder&) = delete;
 
-    ~MTdataHolder() { free_mtdata(m_mtdata); }
+    // Support move semantics.
+    MTdataHolder(MTdataHolder&& h) { std::swap(m_mtdata, h.m_mtdata); }
+    MTdataHolder& operator=(MTdataHolder&& h)
+    {
+        std::swap(m_mtdata, h.m_mtdata);
+        return *this;
+    }
 
-    operator MTdata() const { return m_mtdata; }
+    ~MTdataHolder()
+    {
+        if (m_mtdata) free_mtdata(m_mtdata);
+    }
+
+    operator MTdata() const
+    {
+        assert(m_mtdata && "Object wasn't initialised");
+        return m_mtdata;
+    }
 
 private:
-    MTdata m_mtdata;
+    MTdata m_mtdata = nullptr;
 };
 
 #endif // #ifdef __cplusplus

test_conformance/math_brute_force/binary_double.cpp

@@ -134,7 +134,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     double maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -691,7 +691,7 @@ int TestFunc_Double_Double_Double(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -740,7 +740,7 @@ int TestFunc_Double_Double_Double(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -792,10 +792,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/binary_float.cpp

@@ -132,7 +132,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     double maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -848,7 +848,7 @@ int TestFunc_Float_Float_Float(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -897,7 +897,7 @@ int TestFunc_Float_Float_Float(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -949,10 +949,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/binary_i_double.cpp

@@ -133,7 +133,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     cl_int maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -610,7 +610,7 @@ int TestFunc_Double_Double_Int(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -662,7 +662,7 @@ int TestFunc_Double_Double_Int(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -714,10 +714,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/binary_i_float.cpp

@@ -131,7 +131,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     cl_int maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -603,7 +603,7 @@ int TestFunc_Float_Float_Int(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -655,7 +655,7 @@ int TestFunc_Float_Float_Int(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -707,10 +707,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/binary_operator_double.cpp

@@ -133,7 +133,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     double maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -658,7 +658,7 @@ int TestFunc_Double_Double_Double_Operator(const Func *f, MTdata d,
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -707,7 +707,7 @@ int TestFunc_Double_Double_Double_Operator(const Func *f, MTdata d,
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -759,10 +759,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/binary_operator_float.cpp

@@ -131,7 +131,7 @@ struct ThreadInfo
         maxErrorValue; // position of the max error value (param 1).  Init to 0.
     double maxErrorValue2; // position of the max error value (param 2).  Init
                            // to 0.
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -785,7 +785,7 @@ int TestFunc_Float_Float_Float_Operator(const Func *f, MTdata d,
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -834,7 +834,7 @@ int TestFunc_Float_Float_Float_Operator(const Func *f, MTdata d,
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -886,10 +886,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/macro_binary_double.cpp

@@ -127,7 +127,7 @@ struct ThreadInfo
     clMemWrapper inBuf2;
     Buffers outBuf;
 
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -616,7 +616,7 @@ int TestMacro_Int_Double_Double(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -665,7 +665,7 @@ int TestMacro_Int_Double_Double(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -704,10 +704,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/macro_binary_float.cpp

@@ -125,7 +125,7 @@ struct ThreadInfo
     clMemWrapper inBuf2;
     Buffers outBuf;
 
-    MTdata d;
+    MTdataHolder d;
 
     // Per thread command queue to improve performance
     clCommandQueueWrapper tQueue;
@@ -605,7 +605,7 @@ int TestMacro_Int_Float_Float(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {
@@ -654,7 +654,7 @@ int TestMacro_Int_Float_Float(const Func *f, MTdata d, bool relaxedMode)
             goto exit;
         }
 
-        test_info.tinfo[i].d = init_genrand(genrand_int32(d));
+        test_info.tinfo[i].d = MTdataHolder(genrand_int32(d));
     }
 
     // Init the kernels
@@ -693,10 +693,5 @@ exit:
         }
     }
 
-    for (auto &threadInfo : test_info.tinfo)
-    {
-        free_mtdata(threadInfo.d);
-    }
-
     return error;
 }

test_conformance/math_brute_force/macro_unary_double.cpp

@@ -400,7 +400,7 @@ int TestMacro_Int_Double(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {

test_conformance/math_brute_force/macro_unary_float.cpp

@@ -414,7 +414,7 @@ int TestMacro_Int_Float(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {

test_conformance/math_brute_force/main.cpp

@@ -98,7 +98,7 @@ cl_mem gInBuffer2 = NULL;
 cl_mem gInBuffer3 = NULL;
 cl_mem gOutBuffer[VECTOR_SIZE_COUNT] = { NULL, NULL, NULL, NULL, NULL, NULL };
 cl_mem gOutBuffer2[VECTOR_SIZE_COUNT] = { NULL, NULL, NULL, NULL, NULL, NULL };
-static MTdata gMTdata;
+static MTdataHolder gMTdata;
 cl_device_fp_config gFloatCapabilities = 0;
 int gWimpyReductionFactor = 32;
 int gVerboseBruteForce = 0;
@@ -326,7 +326,7 @@ int main(int argc, const char *argv[])
     vlog("\n-------------------------------------------------------------------"
          "----------------------------------------\n");
 
-    gMTdata = init_genrand(gRandomSeed);
+    gMTdata = MTdataHolder(gRandomSeed);
 
     FPU_mode_type oldMode;
     DisableFTZ(&oldMode);
@@ -336,8 +336,6 @@ int main(int argc, const char *argv[])
 
     RestoreFPState(&oldMode);
 
-    free_mtdata(gMTdata);
-
     if (gQueue)
     {
         int error_code = clFinish(gQueue);

test_conformance/math_brute_force/unary_double.cpp

@@ -427,7 +427,7 @@ int TestFunc_Double_Double(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {

test_conformance/math_brute_force/unary_float.cpp

@@ -580,7 +580,7 @@ int TestFunc_Float_Float(const Func *f, MTdata d, bool relaxedMode)
         test_info.k[i].resize(test_info.threadCount, nullptr);
     }
 
-    test_info.tinfo.resize(test_info.threadCount, ThreadInfo{});
+    test_info.tinfo.resize(test_info.threadCount);
     for (cl_uint i = 0; i < test_info.threadCount; i++)
     {
         cl_buffer_region region = {