[PATCH] D13949: [SLP] Treat SelectInsts as reduction values.

[Nadav Rotem via llvm-commits]

Okay. Please go ahead and commit this patch.  I do have one minor comment:

       BinaryOperator *Next = dyn_cast<BinaryOperator>(NextV);
       if (Next)
         Stack.push_back(std::make_pair(Next, 0));
+      else if (SelectInst *SelI = dyn_cast<SelectInst>(NextV))
+        Stack.push_back(std::make_pair(SelI, 0));
       else if (NextV != Phi)
         return false;
     }

You can rewrite this code using ‘isa<BinaryOperator>(I) || isa<SelectInst>(I)’ and use a single push_back instruction. 

It is also a good idea to add some comments to this code. 


-Nadav

> On Oct 26, 2015, at 6:15 AM, Charlie Turner <charlie.turner at arm.com> wrote:
> 
> chatur01 added a comment.
> 
> Hi Nadav, I've anaysed my patches in more detail now. Sorry for how long it took me to do it!
> 
> This patch doesn't affect compile-time performance in any significant way.
> 
> My testing methodology for LNT was to run the following with and without my changes, pegged on just one A57 CPU:
> 
>  $ taskset -c 5 ./bin/lnt runtest nt --sandbox $SANDBOX --cc=$COMPILER --cxx=$COMPILER '--cflag=-mcpu=cortex-a57 -mllvm -slp-vectorize-hor -mllvm -slp-vectorize-hor-store -Wl,--allow-multiple-definition' --test-suite $TEST_SUITE_DIR --no-timestamp --make-param=ARCH=AArch64 --make-param=ENDIAN=little --make-param=RUNTIMELIMIT=7200 '--make-param=RUNUNDER=taskset -c 5' --multisample=1 --threads 1 --build-threads 1 --benchmarking-only --use-perf 1
> 
> I then collected all the compile time results from the two runs and compared them. The differences were all within noise.
> 
> The methodology for SPEC{2000,2006} was to use their runspec drivers, and to time each benchmark's build "action" from a clean installation with and without my patch. There were also no significant differences in these benchmarks.
> 
> There are no significant differences in performance with this patch across SPEC{2000,2006} and LNT.
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D13949
> 
> 
>