[polly] r244606 - Revise the simplification of regions

Tue Aug 11 14:02:02 PDT 2015

When I execute the test-suite on my local machine, I get some
different error messages, but also for the same programs:

oggenc:
UNREACHABLE executed at
/home/meinersbur/src/llvm-svn/tools/polly/lib/CodeGen/BlockGenerators.cpp:146!

pifft:
TIMING OUT PROCESS: Output/pifft.simple

So I looked into the oggenc case first. The function that is failing
is Laguerre_With_Deflation with the second detected scop:

; Function Attrs: nounwind uwtable
define internal i32 @Laguerre_With_Deflation(float* %a, i32 %ord,
float* %r) #0 {
<snip>

cleanup:                                          ; preds =
%if.end.52, %for.end.24
  %new.1 = phi double [ %new.0, %for.end.24 ], [ %sub55, %if.end.52 ]
  %cleanup.dest.slot.0 = phi i32 [ 1, %for.end.24 ], [ %., %if.end.52 ]
  %retval.2 = phi i32 [ -1, %for.end.24 ], [ %retval.1, %if.end.52 ]
  switch i32 %cleanup.dest.slot.0, label %cleanup.88.loopexit [
    i32 0, label %while.body
    i32 9, label %while.end
  ]

while.end:                                        ; preds = %cleanup
  %retval.2.lcssa122 = phi i32 [ %retval.2, %cleanup ]
  %new.1.lcssa120 = phi double [ %new.1, %cleanup ]
  %conv70 = fptrunc double %new.1.lcssa120 to float
  %10 = add nsw i64 %indvars.iv131, -1
  %arrayidx73 = getelementptr inbounds float, float* %r, i64 %10
  store float %conv70, float* %arrayidx73, align 4
  %cmp75.110 = icmp sgt i64 %indvars.iv131, 0
  br i1 %cmp75.110, label %for.body.77.lr.ph, label %for.end.87

<snip>
}

while.end is the scop's entry block. TempScopInfo ignores the lcssa
PHI nodes because they use no value from inside the region.

The BlockGenerator does does generate code for PHI nodes
(copyPHIInstruction is empty). And when %conv70 is copied, there is no
copied value for %new.1.lcssa120 , resulting in this unreachable.

Before my commit, simplify region would create a new block

polly.entering.block:                             ; preds = %cleanup
  %retval.2.lcssa122 = phi i32 [ %retval.2, %cleanup ]
  %new.1.lcssa120 = phi double [ %new.1, %cleanup ]
  br label %polly.split_new_and_old

that takes all the PHI nodes. But the condition why this block has
been created has nothing to do with the PHI nodes (it is that the
cleanup block has multiple outgoing edges). So I think it is just
chance this just blows happens now.

I can think of the following solutions:

1) Add a function to call in CodeGeneration after region
simplification that removes PHI nodes with edges only from outside of
the region. Not sure whether this can always work because the PHIs are
not necessarily in this entry block and can have multiple incoming
edges, so its value depend on the incoming edge, which is not
statically known.

2) Make TempScopInfo not skip such PHI nodes. It follows that later
passes also must handle such PHI nodes specially.

3) Let copyPHIInstruction detect this situation and create a value for
this. Same problem as for 1) applies.

IMHO 2) is the only working solution, but looks complicated. Maybe
someone else has other ideas?

Michael

2015-08-11 17:32 GMT+02:00 Michael Kruse <llvm at meinersbur.de>:
> Looking into it. I find an assertion fail in the log:
>
> clang: /home/grosser/buildslave/perf-x86_64-penryn-O3-polly-fast/llvm.src/lib/Analysis/ScalarEvolution.cpp:3317:
> const llvm::SCEV* llvm::ScalarEvolution::getSCEV(llvm::Value*):
> Assertion `isSCEVable(V->getType()) && "Value is not SCEVable!"'
> failed.
>
> Michael
>
>
> 2015-08-11 17:08 GMT+02:00 Tobias Grosser <tobias at grosser.es>:
>> On 08/11/2015 04:39 PM, Michael Kruse via llvm-commits wrote:
>>>
>>> Author: meinersbur
>>> Date: Tue Aug 11 09:39:21 2015
>>> New Revision: 244606
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=244606&view=rev
>>> Log:
>>> Revise the simplification of regions
>>>
>>> The previous code had several problems:
>>>
>>> For newly created BasicBlocks it did not (always) call
>>> RegionInfo::setRegionFor in order to update its analysis. At the moment
>>> RegionInfo does not verify its BBMap, but will in the future. This is fixed
>>> by determining the region new BBs belong to and set it accordingly. The new
>>> executeScopConditionally() requires accurate getRegionFor information.
>>>
>>> Which block is created by SplitEdge depends on the incoming and outgoing
>>> edges of the blocks it connects, which makes handling its output more
>>> difficult than it needs to be. Especially for finding which block has been
>>> created an to assign a region to it for the setRegionFor problem above. This
>>> patch uses an implementation for splitEdge that always creates a block
>>> between the predecessor and successor. simplifyRegion has also been
>>> simplified by using SplitBlockPredecessors instead of SplitEdge. Isolating
>>> the entries and exits have been refectored into individual functions.
>>>
>>> Previously simplifyRegion did more than just ensuring that there is only
>>> one entering and one exiting edge. It ensured that the entering block had no
>>> other outgoing edge which was necessary for executeScopConditionally(). Now
>>> the latter uses the alternative splitEdge implementation which can handle
>>> this situation so simplifyRegion really only needs to simplify the region.
>>>
>>> Also, executeScopConditionally assumed that there can be no PHI nodes in
>>> blocks with one incoming edge. This is wrong and LCSSA deliberately produces
>>> such edges. However, previous passes ensured that there can be no such PHIs
>>> in exit nodes, but which will no longer hold in the future.
>>>
>>> The new code that the property that it preserves the identity of region
>>> block (the property that the memory address of the BasicBlock containing the
>>> instructions remains the same; new blocks only contain PHI nodes and a
>>> terminator), especially the entry block. As a result, there is no need to
>>> update the reference to the BasicBlock of ScopStmt that contain its
>>> instructions because they have been moved to other basic blocks.
>>>
>>> Reviewers: grosser
>>>
>>> Part of Differential Revision: http://reviews.llvm.org/D11867
>>
>>
>> It seems this triggers two LNT failures:
>>
>> http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-fast/builds/10539
>>
>> Tobias