Various fixes (memory leak & crash) in the OpenCL interpreter code.

Pre-compiling of OpenCL kernel is disabled with this change.

(cherry picked from commit b981c089a9194f33b46272e3f4efa117241ea533)
(cherry picked from commit e253d46af3b47afd0006084bec89b02473ee457a)
(cherry picked from commit 0d7f89b021f1354a0fa607862296b11c9fd8a4dd)
(cherry picked from commit 5325137783825c498ed4236080ed7fe51cdec09a)
(cherry picked from commit 20ed6886ade81ee015a22b2eb3aeff64691971bf)
(cherry picked from commit 7a0ed3a406bd14bb2b52c383282daf4777ba76e0)

Conflicts:
	sc/source/core/data/formulacell.cxx

Change-Id: Iae97f4f877c329f443c7d222c6a2016678af2387
Reviewed-on: https://gerrit.libreoffice.org/8496
Tested-by: Markus Mohrhard <markus.mohrhard@googlemail.com>
Reviewed-by: Markus Mohrhard <markus.mohrhard@googlemail.com>

8 files changed, 72 insertions, 64 deletions

diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 7361b66c3120..2cfdbb9bc623 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -64,7 +64,9 @@ struct SC_DLLPUBLIC ScFormulaCellGroup : boost::noncopyable
     short mnFormatType;
     bool mbInvariant:1;
     bool mbSubTotal:1;
-    sc::GroupCalcState meCalcState;
+
+    sal_uInt8 meCalcState;
+    sal_uInt8 meKernelState;
 
     ScFormulaCellGroup();
     ~ScFormulaCellGroup();
@@ -74,6 +76,7 @@ struct SC_DLLPUBLIC ScFormulaCellGroup : boost::noncopyable
     void setCode( const ScTokenArray& rCode );
     void compileCode(
         ScDocument& rDoc, const ScAddress& rPos, formula::FormulaGrammar::Grammar eGram );
+    void compileOpenCLKernel();
 
     static int snCount;
     static rtl::Reference<sc::CLBuildKernelThread> sxCompilationThread;
diff --git a/sc/inc/formulagroup.hxx b/sc/inc/formulagroup.hxx
index a3f1891417a4..bb0e0b42e4e4 100644
--- a/sc/inc/formulagroup.hxx
+++ b/sc/inc/formulagroup.hxx
@@ -107,7 +107,7 @@ class SC_DLLPUBLIC FormulaGroupInterpreter
     virtual ScMatrixRef inverseMatrix(const ScMatrix& rMat) = 0;
     virtual CompiledFormula* createCompiledFormula(ScDocument& rDoc,
                                                    const ScAddress& rTopPos,
-                                                   ScFormulaCellGroupRef& xGroup,
+                                                   ScFormulaCellGroup& rGroup,
                                                    ScTokenArray& rCode) = 0;
     virtual bool interpret(ScDocument& rDoc, const ScAddress& rTopPos, ScFormulaCellGroupRef& xGroup, ScTokenArray& rCode) = 0;
 };
@@ -122,7 +122,7 @@ public:
     virtual ScMatrixRef inverseMatrix(const ScMatrix& rMat);
     virtual CompiledFormula* createCompiledFormula(ScDocument& rDoc,
                                                    const ScAddress& rTopPos,
-                                                   ScFormulaCellGroupRef& xGroup,
+                                                   ScFormulaCellGroup& rGroup,
                                                    ScTokenArray& rCode) SAL_OVERRIDE;
     virtual bool interpret(ScDocument& rDoc, const ScAddress& rTopPos, ScFormulaCellGroupRef& xGroup, ScTokenArray& rCode) SAL_OVERRIDE;
 };
diff --git a/sc/inc/types.hxx b/sc/inc/types.hxx
index fc0e0e85c07b..b6c4fa64bb78 100644
--- a/sc/inc/types.hxx
+++ b/sc/inc/types.hxx
@@ -58,11 +58,16 @@ const sal_uInt16 MatrixEdgeOpen    = 32;
 
 enum GroupCalcState
 {
+    GroupCalcDisabled = 0,
     GroupCalcEnabled,
-    GroupCalcOpenCLKernelCompilationScheduled,
-    GroupCalcOpenCLKernelBinaryCreated,
     GroupCalcRunning,
-    GroupCalcDisabled
+};
+
+enum OpenCLKernelState
+{
+    OpenCLKernelNone = 0,
+    OpenCLKernelCompilationScheduled,
+    OpenCLKernelBinaryCreated
 };
 
 struct RangeMatrix
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 71d55f73ed4a..9407fbd5dc72 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -54,6 +54,8 @@
 
 #include <boost/scoped_ptr.hpp>
 
+#define ENABLE_THREADED_OPENCL_KERNEL_COMPILATION 0
+
 using namespace formula;
 
 #ifdef USE_MEMPOOL
@@ -414,7 +416,8 @@ ScFormulaCellGroup::ScFormulaCellGroup() :
     mnFormatType(NUMBERFORMAT_NUMBER),
     mbInvariant(false),
     mbSubTotal(false),
-    meCalcState(sc::GroupCalcEnabled)
+    meCalcState(sc::GroupCalcEnabled),
+    meKernelState(sc::OpenCLKernelNone)
 {
     if (ScInterpreter::GetGlobalConfig().mbOpenCLEnabled)
     {
@@ -447,7 +450,7 @@ ScFormulaCellGroup::~ScFormulaCellGroup()
 
 void ScFormulaCellGroup::scheduleCompilation()
 {
-    meCalcState = sc::GroupCalcOpenCLKernelCompilationScheduled;
+    meKernelState = sc::OpenCLKernelCompilationScheduled;
     sc::CLBuildKernelWorkItem aWorkItem;
     aWorkItem.meWhatToDo = sc::CLBuildKernelWorkItem::COMPILE;
     aWorkItem.mxGroup = this;
@@ -482,7 +485,16 @@ void ScFormulaCellGroup::compileCode(
     }
 }
 
-// ============================================================================
+void ScFormulaCellGroup::compileOpenCLKernel()
+{
+    if (meCalcState == sc::GroupCalcDisabled)
+        return;
+
+    mpCompiledFormula = sc::FormulaGroupInterpreter::getStatic()->createCompiledFormula(
+        *mpTopCell->GetDocument(), mpTopCell->aPos, *this, *mpCode);
+
+    meKernelState = sc::OpenCLKernelBinaryCreated;
+}
 
 ScFormulaCell::ScFormulaCell( ScDocument* pDoc, const ScAddress& rPos ) :
     eTempGrammar(formula::FormulaGrammar::GRAM_DEFAULT),
@@ -2072,7 +2084,7 @@ bool ScFormulaCell::IsMultilineResult()
 
 void ScFormulaCell::MaybeInterpret()
 {
-    if (mxGroup && mxGroup->meCalcState == sc::GroupCalcOpenCLKernelCompilationScheduled)
+    if (mxGroup && mxGroup->meKernelState == sc::OpenCLKernelCompilationScheduled)
         return;
 
     if (!IsDirtyOrInTableOpDirty())
@@ -3509,8 +3521,10 @@ ScFormulaCellGroupRef ScFormulaCell::CreateCellGroup( SCROW nLen, bool bInvarian
     mxGroup->mbInvariant = bInvariant;
     mxGroup->mnLength = nLen;
     mxGroup->mpCode = pCode; // Move this to the shared location.
+#if ENABLE_THREADED_OPENCL_KERNEL_COMPILATION
     if (mxGroup->sxCompilationThread.is())
         mxGroup->scheduleCompilation();
+#endif
     return mxGroup;
 }
 
diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 16f49c6a912c..a478b21ce8a5 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -3103,7 +3103,7 @@ class DynamicKernel : public CompiledFormula
 {
 public:
     DynamicKernel(FormulaTreeNodeRef r):mpRoot(r),
-        mpProgram(NULL), mpKernel(NULL), mpResClmem(NULL), mpCode(NULL) {}
+        mpProgram(NULL), mpKernel(NULL), mpResClmem(NULL) {}
     static DynamicKernel *create(ScDocument& rDoc,
                                  const ScAddress& rTopPos,
                                  ScTokenArray& rCode);
@@ -3198,10 +3198,9 @@ public:
     }
     ~DynamicKernel();
     cl_mem GetResultBuffer(void) const { return mpResClmem; }
-    void SetPCode(ScTokenArray *pCode) { mpCode = pCode; }
 
 private:
-    void TraverseAST(FormulaTreeNodeRef);
+
     FormulaTreeNodeRef mpRoot;
     SymbolTable mSyms;
     std::string mKernelSignature, mKernelHash;
@@ -3211,7 +3210,6 @@ private:
     cl_mem mpResClmem; // Results
     std::set<std::string> inlineDecl;
     std::set<std::string> inlineFun;
-    ScTokenArray *mpCode;
 };
 
 DynamicKernel::~DynamicKernel()
@@ -3225,12 +3223,14 @@ DynamicKernel::~DynamicKernel()
         clReleaseKernel(mpKernel);
     }
     // mpProgram is not going to be released here -- it's cached.
-    if (mpCode)
-        delete mpCode;
 }
 /// Build code
 void DynamicKernel::CreateKernel(void)
 {
+    if (mpKernel)
+        // already created.
+        return;
+
     cl_int err;
     std::string kname = "DynamicKernel"+mKernelSignature;
     // Compile kernel here!!!
@@ -3321,7 +3321,7 @@ public:
     virtual ScMatrixRef inverseMatrix( const ScMatrix& rMat ) SAL_OVERRIDE;
     virtual CompiledFormula* createCompiledFormula(ScDocument& rDoc,
                                                    const ScAddress& rTopPos,
-                                                   ScFormulaCellGroupRef& xGroup,
+                                                   ScFormulaCellGroup& rGroup,
                                                    ScTokenArray& rCode) SAL_OVERRIDE;
     virtual bool interpret( ScDocument& rDoc, const ScAddress& rTopPos,
                             ScFormulaCellGroupRef& xGroup, ScTokenArray& rCode ) SAL_OVERRIDE;
@@ -3337,45 +3337,44 @@ DynamicKernel* DynamicKernel::create(ScDocument& /* rDoc */,
                                      ScTokenArray& rCode)
 {
     // Constructing "AST"
-    FormulaTokenIterator aCode = rCode;
-    std::list<FormulaToken *> list;
-    std::map<FormulaToken *, FormulaTreeNodeRef> m_hash_map;
+    FormulaTokenIterator aCode(rCode);
+    std::list<FormulaToken*> aTokenList;
+    std::map<FormulaToken*, FormulaTreeNodeRef> aHashMap;
     FormulaToken*  pCur;
     while( (pCur = (FormulaToken*)(aCode.Next()) ) != NULL)
     {
         OpCode eOp = pCur->GetOpCode();
         if ( eOp != ocPush )
         {
-            FormulaTreeNodeRef m_currNode =
-                 FormulaTreeNodeRef(new FormulaTreeNode(pCur));
-            sal_uInt8 m_ParamCount =  pCur->GetParamCount();
-            for(int i=0; i<m_ParamCount; i++)
+            FormulaTreeNodeRef pCurNode(new FormulaTreeNode(pCur));
+            sal_uInt8 nParamCount =  pCur->GetParamCount();
+            for (sal_uInt8 i = 0; i < nParamCount; i++)
             {
-                FormulaToken* m_TempFormula = list.back();
-                list.pop_back();
+                FormulaToken* m_TempFormula = aTokenList.back();
+                aTokenList.pop_back();
                 if(m_TempFormula->GetOpCode()!=ocPush)
                 {
-                    if(m_hash_map.find(m_TempFormula)==m_hash_map.end())
+                    if (aHashMap.find(m_TempFormula)==aHashMap.end())
                         return NULL;
-                    m_currNode->Children.push_back(m_hash_map[m_TempFormula]);
+                    pCurNode->Children.push_back(aHashMap[m_TempFormula]);
                 }
                 else
                 {
                     FormulaTreeNodeRef m_ChildTreeNode =
                       FormulaTreeNodeRef(
                                new FormulaTreeNode(m_TempFormula));
-                    m_currNode->Children.push_back(m_ChildTreeNode);
+                    pCurNode->Children.push_back(m_ChildTreeNode);
                 }
             }
-            std::reverse(m_currNode->Children.begin(),
-                                m_currNode->Children.end());
-            m_hash_map[pCur] = m_currNode;
+            std::reverse(pCurNode->Children.begin(),
+                                pCurNode->Children.end());
+            aHashMap[pCur] = pCurNode;
         }
-        list.push_back(pCur);
+        aTokenList.push_back(pCur);
     }
 
     FormulaTreeNodeRef Root = FormulaTreeNodeRef(new FormulaTreeNode(NULL));
-    Root->Children.push_back(m_hash_map[list.back()]);
+    Root->Children.push_back(aHashMap[aTokenList.back()]);
 
     DynamicKernel* pDynamicKernel = new DynamicKernel(Root);
 
@@ -3407,22 +3406,17 @@ DynamicKernel* DynamicKernel::create(ScDocument& /* rDoc */,
 
 CompiledFormula* FormulaGroupInterpreterOpenCL::createCompiledFormula(ScDocument& rDoc,
                                                                       const ScAddress& rTopPos,
-                                                                      ScFormulaCellGroupRef& xGroup,
+                                                                      ScFormulaCellGroup& rGroup,
                                                                       ScTokenArray& rCode)
 {
-    ScTokenArray *pCode = new ScTokenArray();
-    ScGroupTokenConverter aConverter(*pCode, rDoc, *xGroup->mpTopCell, rTopPos);
-    if (!aConverter.convert(rCode) || pCode->GetLen() == 0)
-    {
-        delete pCode;
+    ScTokenArray aConvertedCode;
+    ScGroupTokenConverter aConverter(aConvertedCode, rDoc, *rGroup.mpTopCell, rTopPos);
+    if (!aConverter.convert(rCode) || aConvertedCode.GetLen() == 0)
         return NULL;
-    }
-    SymbolTable::nR = xGroup->mnLength;
 
-    DynamicKernel *result = DynamicKernel::create(rDoc, rTopPos, *pCode);
-    if ( result )
-        result->SetPCode(pCode);
-    return result;
+    SymbolTable::nR = rGroup.mnLength;
+
+    return DynamicKernel::create(rDoc, rTopPos, aConvertedCode);
 }
 
 bool FormulaGroupInterpreterOpenCL::interpret( ScDocument& rDoc,
@@ -3431,10 +3425,10 @@ bool FormulaGroupInterpreterOpenCL::interpret( ScDocument& rDoc,
 {
     DynamicKernel *pKernel;
 
-    if (xGroup->meCalcState == sc::GroupCalcOpenCLKernelCompilationScheduled ||
-        xGroup->meCalcState == sc::GroupCalcOpenCLKernelBinaryCreated)
+    if (xGroup->meKernelState == sc::OpenCLKernelCompilationScheduled ||
+        xGroup->meKernelState == sc::OpenCLKernelBinaryCreated)
     {
-        if (xGroup->meCalcState == sc::GroupCalcOpenCLKernelCompilationScheduled)
+        if (xGroup->meKernelState == sc::OpenCLKernelCompilationScheduled)
         {
             ScFormulaCellGroup::sxCompilationThread->maCompilationDoneCondition.wait();
             ScFormulaCellGroup::sxCompilationThread->maCompilationDoneCondition.reset();
@@ -3445,7 +3439,7 @@ bool FormulaGroupInterpreterOpenCL::interpret( ScDocument& rDoc,
     else
     {
         assert(xGroup->meCalcState == sc::GroupCalcRunning);
-        pKernel = static_cast<DynamicKernel*>(createCompiledFormula(rDoc, rTopPos, xGroup, rCode));
+        pKernel = static_cast<DynamicKernel*>(createCompiledFormula(rDoc, rTopPos, *xGroup, rCode));
     }
 
     if (!pKernel)
diff --git a/sc/source/core/opencl/opbase.hxx b/sc/source/core/opencl/opbase.hxx
index 2f3c7327ef2e..151d4e32cfd4 100644
--- a/sc/source/core/opencl/opbase.hxx
+++ b/sc/source/core/opencl/opbase.hxx
@@ -128,17 +128,17 @@ typedef boost::shared_ptr<FormulaTreeNode> FormulaTreeNodeRef;
 class FormulaTreeNode
 {
 public:
-    FormulaTreeNode(formula::FormulaToken *ft): mpCurrentFormula(ft)
+    FormulaTreeNode(const formula::FormulaToken* ft): mpCurrentFormula(ft)
     {
         Children.reserve(8);
     }
     std::vector<FormulaTreeNodeRef> Children;
     formula::FormulaToken *GetFormulaToken(void) const
     {
-        return mpCurrentFormula;
+        return const_cast<formula::FormulaToken*>(mpCurrentFormula.get());
     }
 private:
-    formula::FormulaToken *const mpCurrentFormula;
+    formula::FormulaConstTokenRef mpCurrentFormula;
 };
 
 /// (Partially) abstract base class for an operand
diff --git a/sc/source/core/tool/clkernelthread.cxx b/sc/source/core/tool/clkernelthread.cxx
index 2a619c5cdbad..ea3c7d0c1331 100644
--- a/sc/source/core/tool/clkernelthread.cxx
+++ b/sc/source/core/tool/clkernelthread.cxx
@@ -49,15 +49,7 @@ void CLBuildKernelThread::execute()
             {
             case CLBuildKernelWorkItem::COMPILE:
                 SAL_INFO("sc.opencl.thread", "told to compile group " << aWorkItem.mxGroup << " (state " << aWorkItem.mxGroup->meCalcState << ") to binary");
-                if (aWorkItem.mxGroup->meCalcState == sc::GroupCalcDisabled)
-                    break;
-                assert(aWorkItem.mxGroup->meCalcState == sc::GroupCalcOpenCLKernelCompilationScheduled);
-                aWorkItem.mxGroup->mpCompiledFormula =
-                    sc::FormulaGroupInterpreter::getStatic()->createCompiledFormula(*aWorkItem.mxGroup->mpTopCell->GetDocument(),
-                                                                                    aWorkItem.mxGroup->mpTopCell->aPos,
-                                                                                    aWorkItem.mxGroup,
-                                                                                    *aWorkItem.mxGroup->mpCode);
-                aWorkItem.mxGroup->meCalcState = sc::GroupCalcOpenCLKernelBinaryCreated;
+                aWorkItem.mxGroup->compileOpenCLKernel();
                 SAL_INFO("sc.opencl.thread", "group " << aWorkItem.mxGroup << " compilation done");
                 maCompilationDoneCondition.set();
                 break;
diff --git a/sc/source/core/tool/formulagroup.cxx b/sc/source/core/tool/formulagroup.cxx
index a611298b7b03..e662195de50d 100644
--- a/sc/source/core/tool/formulagroup.cxx
+++ b/sc/source/core/tool/formulagroup.cxx
@@ -292,7 +292,7 @@ ScMatrixRef FormulaGroupInterpreterSoftware::inverseMatrix(const ScMatrix& /*rMa
 
 CompiledFormula* FormulaGroupInterpreterSoftware::createCompiledFormula(ScDocument& /* rDoc */,
                                                                         const ScAddress& /* rTopPos */,
-                                                                        ScFormulaCellGroupRef& /* xGroup */,
+                                                                        ScFormulaCellGroup& /* rGroup */,
                                                                         ScTokenArray& /* rCode */)
 {
     return NULL;
@@ -500,7 +500,7 @@ public:
     FormulaGroupInterpreterOpenCLMissing() : FormulaGroupInterpreter() {}
     virtual ~FormulaGroupInterpreterOpenCLMissing() {}
     virtual ScMatrixRef inverseMatrix(const ScMatrix&) { return ScMatrixRef(); }
-    virtual CompiledFormula* createCompiledFormula(ScDocument&, const ScAddress&, ScFormulaCellGroupRef&, ScTokenArray&) SAL_OVERRIDE { return NULL; }
+    virtual CompiledFormula* createCompiledFormula(ScDocument&, const ScAddress&, ScFormulaCellGroup&, ScTokenArray&) SAL_OVERRIDE { return NULL; }
     virtual bool interpret(ScDocument&, const ScAddress&, ScFormulaCellGroupRef&,  ScTokenArray&) { return false; }
 };