[PATCH] D100381: [RFC] Improve loop distribute cost model

Tue Apr 13 07:17:34 PDT 2021

sanwou01 added a comment.

In D100381#2685619 <https://reviews.llvm.org/D100381#2685619>, @SjoerdMeijer wrote:

> That certainly looks encouraging.  Just some remarks about the perf numbers first. I found that the llvm test suite can be quite noisy and you certainly need to restrict it to the subset of CTMark for some more meaningful numbers. Just checking, did you do this? Because looking at all tests, I think I see more tests than I would expect with CTMark, but I could be wrong. How about a SPEC run for something that runs a bit longer?

Yeah, the test suite is fairly noisy, even with -DTEST_SUITE_BENCHMARKING_ONLY=On). CTMark is mostly for compile time, isn't it? I don't think it helps much in terms of noise.

In any case, I've only included benchmarks which have distributed loops, but there are changes (noise) of similar size in other tests too. IIRC SPEC was neutral except for the expected gain on hmmer, but I'll re-run them with the current patch.

In D100381#2685652 <https://reviews.llvm.org/D100381#2685652>, @xbolva00 wrote:

> loop dist in hmmer should always work :) and some improvements in TSVC should be expected as well.
>
> https://github.com/UoB-HPC/TSVC_2/blob/master/src/tsvc.c#L1797

I don't think we pick that one up (yet), because it's not the inner loop that needs distribution, which is a limitation of the current pass. It looks like there are a few more in TSVC, so I'll have a look at those.

> Since test suite can be quite noisy, can you check and share for additionally vectorized loops wrt new cost model?

Good idea, I'll get some numbers for that. Loop distribution increases the total number of loops as well, so you'd naturally expect the number of vectorized loops to go up, so it'd be interesting to compare the change in number of loops to the number of *vectorized* loops.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100381/new/

https://reviews.llvm.org/D100381