[PATCH] Choose the best consecutive candidate for a store instruction in SLP vectorizer

[Nadav Rotem]

> On Jun 15, 2015, at 9:20 AM, Wei Mi <wmi at google.com> wrote:
> 
> On Mon, Jun 15, 2015 at 9:10 AM, Nadav Rotem <nrotem at apple.com> wrote:
>> Wei,
>> 
>> Thanks for working on this.  Did you run the llvm test suite? Are there any performance wins or compile time regressions?
> 
> Thanks for the quick review.
> I didn't run llvm testsuite because I found there were some noisy
> benchmarks last time when I run it. Is there a good way to filter
> those noisy benchmarks?

I usually run 3-5 iterations (using LNT), and this reduces the noise. Also, disabling turbo and hyperhreading helps. 
 
The llvm test suite is excellent at catching correctness and performance regressions. I think it is a good idea to run the tests before commits like this. 

> 
>> 
>> I have a few comments below:
>> 
>> -    for (unsigned j = 0; j < e; ++j) {
>> -      if (i == j)
>> -        continue;
>> -      const DataLayout &DL = Stores[i]->getModule()->getDataLayout();
>> +    const DataLayout &DL = Stores[i]->getModule()->getDataLayout();
>> 
>> Please initialize ‘j' when you declare it. Also, why unsigned?
> 
> Because e is unsigned, there will be a warning for j < e if j is not unsigned.
> 
>> 
>> +    unsigned j;
>> +    // If a store has multiple consectutive store candidates, choose
>> +    // the immediate succeeding or preceding one.
>> +    for (j = i + 1; j < e; ++j) {
>> +      if (R.isConsecutiveAccess(Stores[i], Stores[j], DL)) {
>> +        Tails.insert(Stores[j]);
>> +        Heads.insert(Stores[i]);
>> +        ConsecutiveChain[Stores[i]] = Stores[j];
>> +        break;
>> +      }
>> +    }
>> 
>> Please document this line, or simplify it.
>> 
>> +    if (j < e)
>> +      continue;
>> 
>> At this point you are defining a new J variable, with a different type. This is confusing.
>> 
>> +    for (int j = i - 1; j >= 0; --j) {
>> 
>> 
>> Can you think of a way to write this code without code duplication?
>> 
> 
> I will find a way to remove the duplication.
> 
> Thanks,
> Wei.