[Libreoffice-commits] core.git: Branch 'libreoffice-4-2' - 2 commits - sc/inc sc/source

Kohei Yoshida kohei.yoshida at collabora.com
Fri Mar 7 14:39:18 PST 2014


 sc/inc/formulacell.hxx                   |    5 +-
 sc/inc/formulagroup.hxx                  |    4 -
 sc/inc/types.hxx                         |   11 +++-
 sc/source/core/data/formulacell.cxx      |   22 +++++++--
 sc/source/core/opencl/formulagroupcl.cxx |   74 ++++++++++++++-----------------
 sc/source/core/opencl/opbase.hxx         |    6 +-
 sc/source/core/tool/clkernelthread.cxx   |   15 ++----
 sc/source/core/tool/formulagroup.cxx     |    4 -
 8 files changed, 77 insertions(+), 64 deletions(-)

New commits:
commit f41da077c76ee8a70fbcf4fe62e0bfb1fabc1a1c
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Fri Mar 7 11:13:08 2014 -0500

    Initialize OpenCL device on the main thread when pre-compiling kernels.
    
    Else it would cause extremely hard-to-debug random crashes when interpreting
    formulas with OpenCL.
    
    And let's re-enable kernel pre-compilation with this fix.
    
    Change-Id: Ib104bfdc3f56a02c052c837eb4bf3ecc95d562e0
    (cherry picked from commit 6e24c789572aba5b6ee95e894f6d778777b1df58)
    Reviewed-on: https://gerrit.libreoffice.org/8498
    Tested-by: Markus Mohrhard <markus.mohrhard at googlemail.com>
    Reviewed-by: Markus Mohrhard <markus.mohrhard at googlemail.com>

diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 9407fbd..aff135a 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -54,7 +54,7 @@
 
 #include <boost/scoped_ptr.hpp>
 
-#define ENABLE_THREADED_OPENCL_KERNEL_COMPILATION 0
+#define ENABLE_THREADED_OPENCL_KERNEL_COMPILATION 1
 
 using namespace formula;
 
diff --git a/sc/source/core/tool/clkernelthread.cxx b/sc/source/core/tool/clkernelthread.cxx
index ea3c7d0..6c5afc0 100644
--- a/sc/source/core/tool/clkernelthread.cxx
+++ b/sc/source/core/tool/clkernelthread.cxx
@@ -69,6 +69,11 @@ void CLBuildKernelThread::push(CLBuildKernelWorkItem item)
     osl::MutexGuard guard(maQueueMutex);
     maQueue.push(item);
     maQueueCondition.set();
+
+    // This is only to ensure that the OpenCL parameters are initialized on
+    // the main thread before spawning a worker thread for kernel
+    // pre-compilation.
+    sc::FormulaGroupInterpreter::getStatic();
 }
 
 void CLBuildKernelThread::produce()
commit 6048ced1ec27ad64c13791274b66ccb55e4d8dc9
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Wed Mar 5 21:39:10 2014 -0500

    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 at googlemail.com>
    Reviewed-by: Markus Mohrhard <markus.mohrhard at googlemail.com>

diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 7361b66..2cfdbb9 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 a3f1891..bb0e0b4 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 fc0e0e8..b6c4fa6 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 71d55f7..9407fbd 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 16f49c6..a478b21 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 2f3c732..151d4e3 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 2a619c5..ea3c7d0 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 a611298..e662195 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; }
 };
 


More information about the Libreoffice-commits mailing list