[polly] r252301 - Fix reuse of non-dominating synthesized value in subregion exit

Fri Nov 6 21:40:07 PST 2015

This actually caused 24 more failures, see:
  http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable/builds/557

Before we were at 67, after at 91 and after r97986 we are now at 75,
see:
  http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-unprofitable/builds/569

This brings me to several problems I would like to discuss:
  1) Our "fixes" are not checked to be beneficial for more then the test
     case that was attached to the bug report (see above).
  2) The number of people looking at the lnt run and extracting tests is
     to little. Tobias complained in SJ about that, now it just
     shifted..
  3) We try to fix things one by one while the underlying problem is not
     exposed. Lately there are mainly dominance problems in non-affine
     regions (we see the 4th or 5th bug now). While they are all fixed
     by small changes to the code, it seems the changes always only
     fix that test case and sometimes even make things worse (see 1)). I
     would suggest to go back to the drawing board here and revert the
     change that introduced these problems (changed the scalar
     reload/store locations). Especially since we commited it
     preemptively and now might not need it anyways. Please bring
     forward counter arguments to this proposal.
  4) We __all__ have to get our priorities straight. If these bots are
     not green we cannot (or at least should not) commit anything new,
     though there is already new stuff pending for a while now that
     needs to be reviewed and decided on. The longer we wait the more
     work we all have...

Cheers,
  Johannes

On 11/06, Michael Kruse via llvm-commits wrote:
> Author: meinersbur
> Date: Fri Nov  6 07:51:24 2015
> New Revision: 252301
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=252301&view=rev
> Log:
> Fix reuse of non-dominating synthesized value in subregion exit
> 
> We were adding all generated values in non-affine subregions to be used
> for the subregions generated exit block. The thought was that only
> values that are dominating the original exit block can be used there.
> But it is possible for synthesizable values to be expanded in any
> block. If the same values is also used for implicit writes, it would
> try to reuse already synthesized values even if not dominating the exit
> block.
> 
> The fix is to only add values to the list of values usable in the exit
> block only if it is dominating the exit block. This fixes
> llvm.org/PR25412.
> 
> Added:
>     polly/trunk/test/Isl/CodeGen/non-affine-synthesized-in-branch.ll
> Modified:
>     polly/trunk/lib/CodeGen/BlockGenerators.cpp
> 
> Modified: polly/trunk/lib/CodeGen/BlockGenerators.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/CodeGen/BlockGenerators.cpp?rev=252301&r1=252300&r2=252301&view=diff
> ==============================================================================
> --- polly/trunk/lib/CodeGen/BlockGenerators.cpp (original)
> +++ polly/trunk/lib/CodeGen/BlockGenerators.cpp Fri Nov  6 07:51:24 2015
> @@ -1074,7 +1074,8 @@ void RegionGenerator::copyStmt(ScopStmt
>          Blocks.push_back(*SI);
>  
>      // Remember value in case it is visible after this subregion.
> -    ValueMap.insert(RegionMap.begin(), RegionMap.end());
> +    if (DT.dominates(BB, R->getExit()))
> +      ValueMap.insert(RegionMap.begin(), RegionMap.end());
>    }
>  
>    // Now create a new dedicated region exit block and add it to the region map.
> 
> Added: polly/trunk/test/Isl/CodeGen/non-affine-synthesized-in-branch.ll
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/Isl/CodeGen/non-affine-synthesized-in-branch.ll?rev=252301&view=auto
> ==============================================================================
> --- polly/trunk/test/Isl/CodeGen/non-affine-synthesized-in-branch.ll (added)
> +++ polly/trunk/test/Isl/CodeGen/non-affine-synthesized-in-branch.ll Fri Nov  6 07:51:24 2015
> @@ -0,0 +1,30 @@
> +; RUN: opt -polly-process-unprofitable -polly-codegen -S < %s | FileCheck %s
> +;
> +; llvm.org/PR25412
> +; %synthgep caused %gep to be synthesized in subregion_if which was reused for
> +; %retval in subregion_exit, even though it is not dominating subregion_exit.
> +;
> +; CHECK-LABEL: polly.stmt.polly.merge_new_and_old.exit:
> +; CHECK:         %scevgep[[R1:[0-9]*]] = getelementptr %struct.hoge, %struct.hoge* %arg, i64 0, i32 2
> +; CHECK:         store double* %scevgep[[R1]], double** %gep.s2a
> +; CHECK:         br label
> +
> +%struct.hoge = type { double, double, double }
> +
> +define double @func(%struct.hoge* %arg) {
> +entry:
> +  br label %subregion_entry
> +
> +subregion_entry:
> +  %gep = getelementptr inbounds %struct.hoge, %struct.hoge* %arg, i64 0, i32 2
> +  %cond = fcmp ogt double undef, undef
> +  br i1 %cond, label %subregion_if, label %subregion_exit
> +
> +subregion_if:
> +  %synthgep = load double, double* %gep
> +  br label %subregion_exit
> +
> +subregion_exit:
> +  %retval = load double, double* %gep
> +  ret double %retval
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.31

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151107/9b446742/attachment.sig>