[Libreoffice-commits] core.git: 5 commits - formula/source include/formula sc/inc sc/source

Kohei Yoshida kohei.yoshida at collabora.com
Thu Mar 6 17:52:08 PST 2014


 formula/source/core/api/token.cxx        |   11 +++++
 include/formula/token.hxx                |    6 --
 sc/inc/formulacell.hxx                   |    1 
 sc/inc/formulagroup.hxx                  |    4 -
 sc/source/core/data/formulacell.cxx      |   15 +++++++
 sc/source/core/opencl/formulagroupcl.cxx |   64 +++++++++++++------------------
 sc/source/core/opencl/opbase.hxx         |    6 +-
 sc/source/core/tool/clkernelthread.cxx   |   10 ----
 sc/source/core/tool/formulagroup.cxx     |    4 -
 9 files changed, 64 insertions(+), 57 deletions(-)

New commits:
commit 20ed6886ade81ee015a22b2eb3aeff64691971bf
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Thu Mar 6 20:45:25 2014 -0500

    Allow easy toggling of threaded OpenCL kernel compilation.
    
    Via compiler defined macro.
    
    Change-Id: Ic20e6564d99e8ae80c15eda5d12b4dbb76ffbd36

diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 67b0fb6..6f3c555 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -76,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/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 9a328ed..821bc67 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 1
+
 using namespace formula;
 
 #ifdef USE_MEMPOOL
@@ -482,7 +484,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),
@@ -3492,8 +3503,12 @@ 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();
+#else
+    mxGroup->compileOpenCLKernel();
+#endif
     return mxGroup;
 }
 
diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index ea1a7e0..3cf52f5 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -3342,7 +3342,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;
@@ -3427,15 +3427,15 @@ DynamicKernel* DynamicKernel::create(ScDocument& /* rDoc */,
 
 CompiledFormula* FormulaGroupInterpreterOpenCL::createCompiledFormula(ScDocument& rDoc,
                                                                       const ScAddress& rTopPos,
-                                                                      ScFormulaCellGroupRef& xGroup,
+                                                                      ScFormulaCellGroup& rGroup,
                                                                       ScTokenArray& rCode)
 {
     ScTokenArray aConvertedCode;
-    ScGroupTokenConverter aConverter(aConvertedCode, rDoc, *xGroup->mpTopCell, rTopPos);
+    ScGroupTokenConverter aConverter(aConvertedCode, rDoc, *rGroup.mpTopCell, rTopPos);
     if (!aConverter.convert(rCode) || aConvertedCode.GetLen() == 0)
         return NULL;
 
-    SymbolTable::nR = xGroup->mnLength;
+    SymbolTable::nR = rGroup.mnLength;
 
     return DynamicKernel::create(rDoc, rTopPos, aConvertedCode);
 }
@@ -3460,7 +3460,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/tool/clkernelthread.cxx b/sc/source/core/tool/clkernelthread.cxx
index 2715af6..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->meKernelState == sc::OpenCLKernelCompilationScheduled);
-                aWorkItem.mxGroup->mpCompiledFormula =
-                    sc::FormulaGroupInterpreter::getStatic()->createCompiledFormula(*aWorkItem.mxGroup->mpTopCell->GetDocument(),
-                                                                                    aWorkItem.mxGroup->mpTopCell->aPos,
-                                                                                    aWorkItem.mxGroup,
-                                                                                    *aWorkItem.mxGroup->mpCode);
-                aWorkItem.mxGroup->meKernelState = sc::OpenCLKernelBinaryCreated;
+                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 806d2d1..d680235 100644
--- a/sc/source/core/tool/formulagroup.cxx
+++ b/sc/source/core/tool/formulagroup.cxx
@@ -294,7 +294,7 @@ ScMatrixRef FormulaGroupInterpreterSoftware::inverseMatrix(const ScMatrix& /*rMa
 
 CompiledFormula* FormulaGroupInterpreterSoftware::createCompiledFormula(ScDocument& /* rDoc */,
                                                                         const ScAddress& /* rTopPos */,
-                                                                        ScFormulaCellGroupRef& /* xGroup */,
+                                                                        ScFormulaCellGroup& /* rGroup */,
                                                                         ScTokenArray& /* rCode */)
 {
     return NULL;
@@ -502,7 +502,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; }
 };
 
commit 5325137783825c498ed4236080ed7fe51cdec09a
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Thu Mar 6 16:00:34 2014 -0500

    Method declared but never defined nor used.
    
    Change-Id: I3990822cfe202b8537a7c9ad58a61c55bc2e3e82

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index bd8c5eb..ea1a7e0 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -3183,7 +3183,7 @@ public:
     cl_mem GetResultBuffer(void) const { return mpResClmem; }
 
 private:
-    void TraverseAST(FormulaTreeNodeRef);
+
     FormulaTreeNodeRef mpRoot;
     SymbolTable mSyms;
     std::string mKernelSignature, mKernelHash;
commit 0d7f89b021f1354a0fa607862296b11c9fd8a4dd
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Thu Mar 6 15:58:25 2014 -0500

    Let's not leak ScTokenArray when compiling formula & some naming convention.
    
    Names that begin with m_ are NOT to be used as local variables, and let's not
    use a name that's identical to the name of a STL container.
    
    Change-Id: I142cc76e9eb34a19a2346cbb67774d60b801a649

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index ec727a8..bd8c5eb 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -3358,45 +3358,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);
 
@@ -3431,16 +3430,14 @@ CompiledFormula* FormulaGroupInterpreterOpenCL::createCompiledFormula(ScDocument
                                                                       ScFormulaCellGroupRef& xGroup,
                                                                       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, *xGroup->mpTopCell, rTopPos);
+    if (!aConverter.convert(rCode) || aConvertedCode.GetLen() == 0)
         return NULL;
-    }
+
     SymbolTable::nR = xGroup->mnLength;
 
-    return DynamicKernel::create(rDoc, rTopPos, *pCode);
+    return DynamicKernel::create(rDoc, rTopPos, aConvertedCode);
 }
 
 bool FormulaGroupInterpreterOpenCL::interpret( ScDocument& rDoc,
diff --git a/sc/source/core/opencl/opbase.hxx b/sc/source/core/opencl/opbase.hxx
index f5ceac5..a75e160 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
commit 0ca7ab6ea027d58e5bfb7be14508f4cf45d85876
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Thu Mar 6 15:01:39 2014 -0500

    Make these non inline.
    
    Change-Id: Ib139850380b039382565e3de3666eebb5750f32d

diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index 9ea95f9..4f8b8aa 100644
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -60,6 +60,17 @@ inline bool lcl_IsReference( OpCode eOp, StackVar eType )
 }
 
 // --- class FormulaToken --------------------------------------------------------
+
+FormulaToken::FormulaToken( StackVar eTypeP, OpCode e ) :
+    eOp(e), eType( eTypeP ), mnRefCnt(0)
+{
+}
+
+FormulaToken::FormulaToken( const FormulaToken& r ) :
+    IFormulaToken(), eOp(r.eOp), eType( r.eType ), mnRefCnt(0)
+{
+}
+
 FormulaToken::~FormulaToken()
 {
 }
diff --git a/include/formula/token.hxx b/include/formula/token.hxx
index baf050a..f65220b 100644
--- a/include/formula/token.hxx
+++ b/include/formula/token.hxx
@@ -94,10 +94,8 @@ protected:
             mutable oslInterlockedCount mnRefCnt;        // reference count
 
 public:
-                                FormulaToken( StackVar eTypeP,OpCode e = ocPush ) :
-                                    eOp(e), eType( eTypeP ), mnRefCnt(0) {}
-                                FormulaToken( const FormulaToken& r ) : IFormulaToken(),
-                                    eOp(r.eOp), eType( r.eType ), mnRefCnt(0) {}
+    FormulaToken( StackVar eTypeP,OpCode e = ocPush );
+    FormulaToken( const FormulaToken& r );
 
     virtual                     ~FormulaToken();
 
commit e253d46af3b47afd0006084bec89b02473ee457a
Author: Kohei Yoshida <kohei.yoshida at collabora.com>
Date:   Thu Mar 6 09:22:56 2014 -0500

    This ScTokenArray is stored but not used. Remove it.
    
    Not only that, it attempts to delete a non-cloned copy in the dtor, which
    would lead to double deletion.
    
    Change-Id: I3ce5266e894354a8cac5dffb5f350b942f463159

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index de4b6e6..ec727a8 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -3088,7 +3088,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);
@@ -3181,7 +3181,6 @@ public:
     }
     ~DynamicKernel();
     cl_mem GetResultBuffer(void) const { return mpResClmem; }
-    void SetPCode(ScTokenArray *pCode) { mpCode = pCode; }
 
 private:
     void TraverseAST(FormulaTreeNodeRef);
@@ -3194,7 +3193,6 @@ private:
     cl_mem mpResClmem; // Results
     std::set<std::string> inlineDecl;
     std::set<std::string> inlineFun;
-    ScTokenArray *mpCode;
 };
 
 DynamicKernel::~DynamicKernel()
@@ -3206,8 +3204,6 @@ 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)
@@ -3444,10 +3440,7 @@ CompiledFormula* FormulaGroupInterpreterOpenCL::createCompiledFormula(ScDocument
     }
     SymbolTable::nR = xGroup->mnLength;
 
-    DynamicKernel *result = DynamicKernel::create(rDoc, rTopPos, *pCode);
-    if ( result )
-        result->SetPCode(pCode);
-    return result;
+    return DynamicKernel::create(rDoc, rTopPos, *pCode);
 }
 
 bool FormulaGroupInterpreterOpenCL::interpret( ScDocument& rDoc,


More information about the Libreoffice-commits mailing list