[Libreoffice-commits] core.git: 2 commits - sc/source

Tor Lillqvist tml at collabora.com
Tue Aug 25 05:48:39 PDT 2015


 sc/source/core/opencl/formulagroupcl.cxx |   48 ++++++++++++--
 sc/source/core/opencl/op_spreadsheet.cxx |  102 ++++++++++++++++---------------
 2 files changed, 96 insertions(+), 54 deletions(-)

New commits:
commit d4310b6cd8a367666cc702c3e47cf12a35407ef7
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Aug 25 15:38:58 2015 +0300

    Cosmetics
    
    Try to use some sane consistent formatting in this function. No semantic
    change.
    
    Change-Id: Ic9e4625c910f826246451e8ff9e18d6131c04a78

diff --git a/sc/source/core/opencl/op_spreadsheet.cxx b/sc/source/core/opencl/op_spreadsheet.cxx
index 3b06460..c005d2a 100644
--- a/sc/source/core/opencl/op_spreadsheet.cxx
+++ b/sc/source/core/opencl/op_spreadsheet.cxx
@@ -39,21 +39,23 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
     ss << "    double intermediate = DBL_MAX;\n";
     ss << "    int singleIndex = gid0;\n";
     ss << "    int rowNum = -1;\n";
+
     GenTmpVariables(ss,vSubArguments);
     int arg=0;
     CheckSubArgumentIsNan(ss,vSubArguments,arg++);
     int secondParaWidth = 1;
-    if(vSubArguments[1]->GetFormulaToken()->GetType() ==
-    formula::svDoubleVectorRef)
+
+    if (vSubArguments[1]->GetFormulaToken()->GetType() == formula::svDoubleVectorRef)
     {
         FormulaToken *tmpCur = vSubArguments[1]->GetFormulaToken();
-        const formula::DoubleVectorRefToken*pCurDVR= static_cast<const
-            formula::DoubleVectorRefToken *>(tmpCur);
+        const formula::DoubleVectorRefToken*pCurDVR = static_cast<const formula::DoubleVectorRefToken *>(tmpCur);
         secondParaWidth = pCurDVR->GetArrays().size();
     }
-    arg+=secondParaWidth;
+
+    arg += secondParaWidth;
     CheckSubArgumentIsNan(ss,vSubArguments,arg++);
-    if(vSubArguments.size() == (unsigned int)(3+(secondParaWidth-1)))
+
+    if (vSubArguments.size() == (unsigned int)(3+(secondParaWidth-1)))
     {
         ss << "    double tmp";
         ss << 3+(secondParaWidth-1);
@@ -64,53 +66,57 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
         CheckSubArgumentIsNan(ss,vSubArguments,arg++);
     }
 
-    if(vSubArguments[1]->GetFormulaToken()->GetType() ==
-    formula::svDoubleVectorRef)
+    if (vSubArguments[1]->GetFormulaToken()->GetType() == formula::svDoubleVectorRef)
     {
         FormulaToken *tmpCur = vSubArguments[1]->GetFormulaToken();
-        const formula::DoubleVectorRefToken*pCurDVR= static_cast<const
-            formula::DoubleVectorRefToken *>(tmpCur);
-        size_t nCurWindowSize = pCurDVR->GetArrayLength() <
-        pCurDVR->GetRefRowSize() ? pCurDVR->GetArrayLength():
-        pCurDVR->GetRefRowSize() ;
+        const formula::DoubleVectorRefToken*pCurDVR = static_cast<const formula::DoubleVectorRefToken *>(tmpCur);
+        size_t nCurWindowSize = pCurDVR->GetArrayLength() < pCurDVR->GetRefRowSize() ? pCurDVR->GetArrayLength() : pCurDVR->GetRefRowSize() ;
         int unrollSize = 8;
         ss << "    int loop;\n";
-        if (!pCurDVR->IsStartFixed() && pCurDVR->IsEndFixed()) {
+        if (!pCurDVR->IsStartFixed() && pCurDVR->IsEndFixed())
+        {
             ss << "    loop = ("<<nCurWindowSize<<" - gid0)/";
             ss << unrollSize<<";\n";
 
-        } else if (pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed()) {
+        }
+        else if (pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
+        {
             ss << "    loop = ("<<nCurWindowSize<<" + gid0)/";
             ss << unrollSize<<";\n";
 
-        } else {
+        }
+        else
+        {
             ss << "    loop = "<<nCurWindowSize<<"/"<< unrollSize<<";\n";
         }
 
-        for(int i=0;i<secondParaWidth;i++)
+        for (int i = 0; i < secondParaWidth; i++)
         {
-
             ss << "    for ( int j = 0;j< loop; j++)\n";
             ss << "    {\n";
             ss << "        int i = ";
-            if (!pCurDVR->IsStartFixed()&& pCurDVR->IsEndFixed()) {
-               ss << "gid0 + j * "<< unrollSize <<";\n";
-            }else {
-               ss << "j * "<< unrollSize <<";\n";
+            if (!pCurDVR->IsStartFixed()&& pCurDVR->IsEndFixed())
+            {
+                ss << "gid0 + j * "<< unrollSize <<";\n";
             }
-            if(!pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
+            else
             {
-               ss << "        int doubleIndex = i+gid0;\n";
-            }else
+                ss << "j * "<< unrollSize <<";\n";
+            }
+            if (!pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
             {
-               ss << "        int doubleIndex = i;\n";
+                ss << "        int doubleIndex = i+gid0;\n";
+            }
+            else
+            {
+                ss << "        int doubleIndex = i;\n";
             }
             ss << "        if(tmp";
             ss << 3+(secondParaWidth-1);
             ss << " == 1)\n";
             ss << "        {\n";
 
-            for(int j =0;j < unrollSize;j++)
+            for (int j = 0;j < unrollSize; j++)
             {
                 CheckSubArgumentIsNan(ss,vSubArguments,1+i);
 
@@ -131,7 +137,7 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
 
             ss << "        }else\n";
             ss << "        {\n";
-            for(int j =0;j < unrollSize;j++)
+            for (int j = 0; j < unrollSize; j++)
             {
                 CheckSubArgumentIsNan(ss,vSubArguments,1+i);
 
@@ -149,15 +155,14 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
             ss << "    }\n";
             ss << "    if(rowNum!=-1)\n";
             ss << "    {\n";
-            for(int j=0;j< secondParaWidth; j++)
+            for (int j = 0; j < secondParaWidth; j++)
             {
-
                 ss << "        if(tmp";
                 ss << 2+(secondParaWidth-1);
                 ss << " == ";
                 ss << j+1;
                 ss << ")\n";
-                if( !(vSubArguments[1+j]->IsMixedArgument()))
+                if (!(vSubArguments[1+j]->IsMixedArgument()))
                 {
                     ss << "        {\n";
                     ss << "            tmp = ";
@@ -183,21 +188,27 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
             ss << "        return tmp;\n";
             ss << "    }\n";
             ss << "    for (int i = ";
-            if (!pCurDVR->IsStartFixed() && pCurDVR->IsEndFixed()) {
-               ss << "gid0 + loop *"<<unrollSize<<"; i < ";
-               ss << nCurWindowSize <<"; i++)\n";
-            } else if (pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed()) {
-               ss << "0 + loop *"<<unrollSize<<"; i < gid0+";
-               ss << nCurWindowSize <<"; i++)\n";
-            } else {
-               ss << "0 + loop *"<<unrollSize<<"; i < ";
-               ss << nCurWindowSize <<"; i++)\n";
+            if (!pCurDVR->IsStartFixed() && pCurDVR->IsEndFixed())
+            {
+                ss << "gid0 + loop *"<<unrollSize<<"; i < ";
+                ss << nCurWindowSize <<"; i++)\n";
+            }
+            else if (pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
+            {
+                ss << "0 + loop *"<<unrollSize<<"; i < gid0+";
+                ss << nCurWindowSize <<"; i++)\n";
+            }
+            else
+            {
+                ss << "0 + loop *"<<unrollSize<<"; i < ";
+                ss << nCurWindowSize <<"; i++)\n";
             }
             ss << "    {\n";
-            if(!pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
+            if (!pCurDVR->IsStartFixed() && !pCurDVR->IsEndFixed())
             {
                ss << "        int doubleIndex = i+gid0;\n";
-            }else
+            }
+            else
             {
                ss << "        int doubleIndex = i;\n";
             }
@@ -232,16 +243,15 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
             ss << "    if(rowNum!=-1)\n";
             ss << "    {\n";
 
-            for(int j=0;j< secondParaWidth; j++)
+            for (int j = 0; j < secondParaWidth; j++)
             {
-
                 ss << "        if(tmp";
                 ss << 2+(secondParaWidth-1);
                 ss << " == ";
                 ss << j+1;
                 ss << ")\n";
                 ///Add MixedArguments for string support in Vlookup.
-                if( !(vSubArguments[1+j]->IsMixedArgument()))
+                if (!(vSubArguments[1+j]->IsMixedArgument()))
                 {
                     ss << "            tmp = ";
                     vSubArguments[1+j]->GenDeclRef(ss);
@@ -257,13 +267,11 @@ void OpVLookup::GenSlidingWindowFunction(std::stringstream &ss,
                     vSubArguments[1+j]->GenStringDeclRef(ss);
                     ss << "[rowNum];\n";
                 }
-
             }
             ss << "        return tmp;\n";
             ss << "    }\n";
 
         }
-
     }
     else
     {
commit 7e91726f4d81f0ab27d79ee231abd666b4999758
Author: Tor Lillqvist <tml at collabora.com>
Date:   Tue Aug 25 15:11:54 2015 +0300

    Treat an array of null string pointers as no strings for OpenCL
    
    For some reason, at least in the case of the "Test OpenCL" thing, we get here
    an mpStringArray that is non-null but where all the elements (rtl_uString
    pointers) in it are null. Treat that case as if the mpStringArray was null.
    
    This makes the tests "Test OpenCL" actually use OpenCL. Maybe it has other
    useful effects, too. (But for normal spreadsheet use, the mpStringArray that
    gets handled here *is* null when all the cells used by a formula group are
    numbers. At least it seemed so in a simple test.)
    
    Also add more useful (?) SAL_INFO calls in the area.
    
    Change-Id: I1388786a3a0765af6eb01a63da31e5b83c7a616d

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index caf5687..53834d7 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -152,6 +152,19 @@ std::string linenumberify(const std::string& s)
 }
 #endif
 
+bool AllStringsAreNull(const rtl_uString* const* pStringArray, size_t nLength)
+{
+    if (pStringArray == nullptr)
+        return true;
+
+    for (size_t i = 0; i < nLength; i++)
+        if (pStringArray[i] != nullptr)
+            return false;
+
+    return true;
+}
+
+
 } // anonymous namespace
 
 /// Map the buffer used by an argument and do necessary argument setting
@@ -2615,8 +2628,10 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
 
                     for (size_t j = 0; j < pDVR->GetArrays().size(); ++j)
                     {
-                        SAL_INFO("sc.opencl", "j=" << j << " mpNumericArray=" << pDVR->GetArrays()[j].mpNumericArray <<
+                        SAL_INFO("sc.opencl", "i=" << i << " j=" << j <<
+                                 " mpNumericArray=" << pDVR->GetArrays()[j].mpNumericArray <<
                                  " mpStringArray=" << pDVR->GetArrays()[j].mpStringArray <<
+                                 " allStringsAreNull=" << (AllStringsAreNull(pDVR->GetArrays()[j].mpStringArray, pDVR->GetArrayLength())?"YES":"NO") <<
                                  " takeNumeric=" << (pCodeGen->takeNumeric()?"YES":"NO") <<
                                  " takeString=" << (pCodeGen->takeString()?"YES":"NO"));
 
@@ -2629,6 +2644,8 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                                 pDVR->GetArrays()[j].mpStringArray &&
                                 pCodeGen->takeString())
                             {
+                                // Function takes numbers or strings, there are both
+                                SAL_INFO("sc.opencl", "Numbers and strings and that is OK");
                                 mvSubArguments.push_back(
                                     DynamicKernelArgumentRef(
                                         new DynamicKernelMixedSlidingArgument(mCalcConfig,
@@ -2636,16 +2653,22 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                             }
                             else
                             {
+                                // Not sure I can figure out what case this exactly is;)
+                                SAL_INFO("sc.opencl", "The other case");
                                 mvSubArguments.push_back(
                                     DynamicKernelArgumentRef(VectorRefFactory<VectorRef>(mCalcConfig,
                                             ts, ft->Children[i], mpCodeGen, j)));
                             }
                         }
                         else
+                        {
+                            // Ditto here. This is such crack.
+                            SAL_INFO("sc.opencl", "The outer other case (can't figure out what it exactly means)");
                             mvSubArguments.push_back(
                                 DynamicKernelArgumentRef(VectorRefFactory
                                     <DynamicKernelStringArgument>(mCalcConfig,
                                         ts, ft->Children[i], mpCodeGen, j)));
+                        }
                     }
                 }
                 else if (pChild->GetType() == formula::svSingleVectorRef)
@@ -2653,8 +2676,10 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                     const formula::SingleVectorRefToken* pSVR =
                         static_cast<const formula::SingleVectorRefToken*>(pChild);
 
-                    SAL_INFO("sc.opencl", "mpNumericArray=" << pSVR->GetArray().mpNumericArray <<
+                    SAL_INFO("sc.opencl", "i=" << i <<
+                             " mpNumericArray=" << pSVR->GetArray().mpNumericArray <<
                              " mpStringArray=" << pSVR->GetArray().mpStringArray <<
+                             " allStringsAreNull=" << (AllStringsAreNull(pSVR->GetArray().mpStringArray, pSVR->GetArrayLength())?"YES":"NO") <<
                              " takeNumeric=" << (pCodeGen->takeNumeric()?"YES":"NO") <<
                              " takeString=" << (pCodeGen->takeString()?"YES":"NO"));
 
@@ -2664,17 +2689,19 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                         pCodeGen->takeString())
                     {
                         // Function takes numbers or strings, there are both
+                        SAL_INFO("sc.opencl", "Numbers and strings and that is OK");
                         mvSubArguments.push_back(
                             DynamicKernelArgumentRef(new DynamicKernelMixedArgument(mCalcConfig,
                                     ts, ft->Children[i])));
                     }
                     else if (pSVR->GetArray().mpNumericArray &&
                         pCodeGen->takeNumeric() &&
-                        (pSVR->GetArray().mpStringArray == NULL || mCalcConfig.meStringConversion == ScCalcConfig::StringConversion::ZERO))
+                             (AllStringsAreNull(pSVR->GetArray().mpStringArray, pSVR->GetArrayLength()) || mCalcConfig.meStringConversion == ScCalcConfig::StringConversion::ZERO))
                     {
                         // Function takes numbers, and either there
                         // are no strings, or there are strings but
                         // they are to be treated as zero
+                        SAL_INFO("sc.opencl", "Maybe strings even if want numbers but should be treated as zero");
                         mvSubArguments.push_back(
                             DynamicKernelArgumentRef(new VectorRef(mCalcConfig, ts,
                                     ft->Children[i])));
@@ -2686,6 +2713,7 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                     {
                         // Function takes numbers, and there are only
                         // strings, but they are to be treated as zero
+                        SAL_INFO("sc.opencl", "Only strings even if want numbers but should be treated as zero");
                         mvSubArguments.push_back(
                             DynamicKernelArgumentRef(new VectorRef(mCalcConfig, ts,
                                     ft->Children[i])));
@@ -2693,28 +2721,32 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                     else if (pSVR->GetArray().mpStringArray &&
                         pCodeGen->takeString())
                     {
-                        // There are strings, and the function takes
-                        // strings.
-
+                        // There are strings, and the function takes strings.
+                        SAL_INFO("sc.opencl", "Strings only");
                         mvSubArguments.push_back(
                             DynamicKernelArgumentRef(new DynamicKernelStringArgument(mCalcConfig,
                                     ts, ft->Children[i])));
                     }
-                    else if (pSVR->GetArray().mpStringArray == NULL &&
+                    else if (AllStringsAreNull(pSVR->GetArray().mpStringArray, pSVR->GetArrayLength()) &&
                         pSVR->GetArray().mpNumericArray == NULL)
                     {
                         // There are only empty cells. Push as an
                         // array of NANs
+                        SAL_INFO("sc.opencl", "Only empty cells");
                         mvSubArguments.push_back(
                             DynamicKernelArgumentRef(new VectorRef(mCalcConfig, ts,
                                     ft->Children[i])));
                     }
                     else
+                    {
+                        SAL_INFO("sc.opencl", "Fallback case, rejecting for OpenCL");
                         throw UnhandledToken(pChild,
                             "Got unhandled case here", __FILE__, __LINE__);
+                    }
                 }
                 else if (pChild->GetType() == formula::svDouble)
                 {
+                    SAL_INFO("sc.opencl", "Constant number (?) case");
                     mvSubArguments.push_back(
                         DynamicKernelArgumentRef(new DynamicKernelConstantArgument(mCalcConfig, ts,
                                 ft->Children[i])));
@@ -2722,12 +2754,14 @@ DynamicKernelSoPArguments::DynamicKernelSoPArguments(const ScCalcConfig& config,
                 else if (pChild->GetType() == formula::svString
                     && pCodeGen->takeString())
                 {
+                    SAL_INFO("sc.opencl", "Constant string (?) case");
                     mvSubArguments.push_back(
                         DynamicKernelArgumentRef(new ConstStringArgument(mCalcConfig, ts,
                                 ft->Children[i])));
                 }
                 else
                 {
+                    SAL_INFO("sc.opencl", "Fallback case, rejecting for OpenCL");
                     throw UnhandledToken(pChild, ("unhandled operand " + StackVarEnumToString(pChild->GetType()) + " for ocPush").c_str());
                 }
                 break;


More information about the Libreoffice-commits mailing list