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

[Wei Mi]

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 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.