[PATCH] D41692: [Polly][WIP] Remove immediate dominator heuristic for error block detection.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In https://reviews.llvm.org/D41692#1052643, @Meinersbur wrote:

> I ran a comparison on the test-suite and here is the result:
>
>   Program                                                                        leone_N20_polly  leone_N30_errheuristic diff
>    SingleSource/Benchmarks/Misc/flops                                            4.332            6.311                    45.7% 
>    S...urce/Benchmarks/Polybench/linear-algebra/solvers/gramschmidt/gramschmidt  1.443            1.637                    13.4% 
>    Polybench/Polybench-421/stencils/adi/P421_adi                                21.096           19.344                    -8.3%  
>    External/SPEC/CFP2017rate/507.cactuBSSN_r/507.cactuBSSN_r                    31.899           33.395                     4.7% 
>


Is there a change the number of statically discarded SCoPs due to error blocks (or alternatively a compile time impact)?

> It shows that e.g. Misc/flops runs 45% slower with this patch (comparing medians of three runs). It is, however, the same performance as -O3 run, i.e. Polly makes it slower.
>  507.cactuBSSN_r shows up I think because Polly is able to tile it, but the default size is ... not a good one for this benchmark. That benchmark should also need improvement in the delinearizer, so I an wondering why it even shows up here.
> 
> I proposed this patch because I do not see the idea behind this heuristic. It basically only adds an exception for the first condition in a loop body. What makes the first condition so different than the second? IMHO if a conditional contains a (non-pure/modelable) function call, we should assume that it is not executed because the call would serialize all dependencies in the loops it is in (this only hope would be that the isl rescheduler unswitches the loop). Or maybe I misunderstood the idea behind the heuristic? If so, could you explain the intention?

That is why I wrote the comment (below). One possible improvement is (as explained) the post dominator tree. In addition I have patches that looks at the CFG more closely and also consider the already taken error blocks during detection. I'll have to look for them though and send them out via email.

  // FIXME: This is a simple heuristic to determine if the load is executed
  //        in a conditional. However, we actually would need the control
  //        condition, i.e., the post dominance frontier. Alternatively we
  //        could walk up the dominance tree until we find a block that is
  //        not post dominated by the load and check if it is a conditional
  //        or a loop header.



> I personally would love to see more intelligent heuristics.

Me too.


Repository:
  rPLO Polly

https://reviews.llvm.org/D41692