[polly] scop detection: detect regions bottom up

[Star Tan]

At 2013-06-13 16:55:32,"Tobias Grosser" <tobias at grosser.es> wrote:

>On 06/12/2013 10:27 PM, Star Tan wrote:
>> Hi Sebastian,
>>
>>
>> Your work is cool. I am also working on reducing Polly compile-time for my GSOC 2013 project (https://gist.github.com/tanstar/5508153)
>> I think we can work together by exchanging our ideas ~
>
>Yes, it is great that we got some traction in this direction and that we 
>can work on this collaboratively.
Since Sebastian is working on reducing ScopDetect compile time, I want to adjust my GSOC project plan to investigate this pass recently. In that way, I can exchange my ideas with Sebastian and learn from him. Do you agree, Tobias?
>
>> At 2013-06-13 05:40:31,"Sebastian Pop" <spop at codeaurora.org> wrote:
>>
>>> Tobias Grosser wrote:
>>>> Also, in terms of speeding up scop detection. My guess is that the
>>>> largest speedup would come from detecting scops bottom up, starting
>>>> from small regions to bigger regions. We could then skip analyzing
>>>> regions as soon as we found a construct in a smaller region that
>>>> would
>>>> also invalidate all regions containing the smaller region.
>>>
>>> Here is a patch that makes scop detection to work bottom up.  On my huge C++
>>> benchmark, the attached patch gets the overall sequential time spent in Polly
>>>from 2151 seconds to 1573 seconds.
>> Cool, the compile time is reduced by about 1/4. Could you tell me which benchmark do you use? Was it in LLVM test-suite?
I am interested in this data because Polly compile time seems strongly dependent on benchmarks. 
In my previous GSOC proposal (https://gist.github.com/tanstar/5508153), I have posted the top 15 expensive compile passes and the ScopDetect pass only accounts for 2.1% for compiling doitgen.ll (Table 3) and 6.9% for compiling jmemmgr.ll (Table 4).  However, Sebastian's testing shows that his patch file to the ScopDetect pass can reduce the compile time by 25%! Obviously, his testing benchmark has some special features that make Polly spend a lot of time on detecting Scop regions. I guess Sebastian's testing benchmark has a large number of complex scop regions, but these scops are finally judged as invalid in Polly.
>> I have set up a LNT-based Polly performance tester using LLVM test-suite. Maybe I can help to evaluate your patch on these benchmarks tonight :)
>
>Yes, please. It would be very valuable to get performance numbers on the 
>open source test-suite.
>
>Just to see if this is beneficial in all cases or if there are cases 
>where going the other direction is actually better.
OK, I can have a whole test tonight. However, there are two problems now:
First, testing the whole LLVM test-suite is too expensive. It usually takes about two hours for each 3-samples test. Currently, I have to use my own notebook to test it, so I think we should mainly focused on PolyBench and only have a complete test per week.
Second, I have not found a good way to share my testing results. I currently maintain the testing results in my local LNT-base database. My idea is that I can post the ten best cases and ten worst cases. 
Do you have any other suggestions?
> >>> Also note that there still are redundant computations in the case of nested >>> valid regions: we would start calling isValidRegion on the innermost region >>> containing a loop, then supposing that the parent region is composed of only >>> valid regions, we would call again isValidRegion and that would walk again over >>> all the subregions, validating the CFG, loops, and every instruction again. >>> >>> With the attached patch there are 4 fails: >>> >>> Failing Tests (4): >>>     Polly :: polybench/linear-algebra/solvers/lu/lu_with_param.ll >>>     Polly :: polybench/linear-algebra/solvers/lu/lu_without_param.ll >>>     Polly :: polybench/linear-algebra/solvers/ludcmp/ludcmp_without_param.ll >>>     Polly :: polybench/stencils/jacobi-2d-imper/jacobi-2d-imper_with_param.ll >>> >>> These fails are due to the fact that we do not detect a maximal region anymore: >>> first, scop detection splits region exit blocks when the exit block has multiple >>> exit edges, and then !
 these new basic blocks that we inserted do not belong to >>> any region, and so we end up not discovering a maximal region anymore. >>> >>> I think we can add the newly created blocks to the region info. >>> >>> Another solution, and probably more difficult, is to avoid invalidating the >>> region info by trying to not split the blocks ending a region.  I'm not sure >>> about the impact of such a change on the codegen region versioning. >> Can I do anything to help you? Maybe I can help to fix this problem by avoiding invalidating the region info. > >I let Sebastian answer this, but here some advices from me: > >1) Feel free to review patches > >Reviewing patches is very important and I encourage you to review the  >patches Sebastian posts. Even if you are not an expert in every detail,  >just review the parts of the patch that you understand and ask for parts  >that are not clear. This either shows weaknesses in the patch or it will  >help you to get more into Polly. > >2)!
  Test coverage > >For some patches you may ask yourself if they actual
ly work as intended  >on a certain corner case. In case you find a case you have been unsure  >about, please just write a test case and propose it for inclusion. Such  >tests are very valuable when looking at the code again in the future.  >Also, when reviewing patches, one of the important points is to mentally  >go through the patch and ask yourself if you can write a test case that  >would break the patch. If we can not find one, the patch is >probably ready to go in.
Thanks a lot for your advice.
Yes, I am recently trying to review patch files posted in this llvm-commits mail list ~
>
>Cheers,
>Tobi
Star Tan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130613/7b51f87b/attachment.html>