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

[Wei Mi]

Thanks for the suggestion. I will run the testsuite and get back.

Wei.

On Mon, Jun 15, 2015 at 9:23 AM, Nadav Rotem <nrotem at apple.com> wrote:
>
>> 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.
>