[polly] r183025 - Test that independent block pass does not transform induction variables
Star Tan
tanmx_star at yeah.net
Mon Jun 10 08:37:16 PDT 2013
At 2013-06-10 22:51:26,"Tobias Grosser" <tobias at grosser.es> wrote:
>On 06/10/2013 07:08 AM, Star Tan wrote:
>> Hi Tobias,
>>
>>
>> According to your suggestion, I have fixed this problem by refusing scops, in which a loop header is part of the scop, but the entire loop is not. Please find the attached patch file and revise it.
>
>Thanks!
>
>> 0001-ScopDetect-refuse-regions-that-contain-only-part-of-.patch
>>
>>
>> From 3e6bbcbcaa997c9d65c7627d240137e665317d3b Mon Sep 17 00:00:00 2001
>> From: Star Tan<tanmx_star at yeah.net>
>> Date: Mon, 10 Jun 2013 21:14:47 +0800
>> Subject: [PATCH] ScopDetect: refuse scops that contain only part of a loop.
>>
>> With SCEV, we have to refuse to scops, in which a loop header is part of the
>> scop, but the entire loop is not. Otherwise it will cause error in some
>> testcases, such as:
>> test/IndependentBlocks/indvars.ll
>> test/ScopDetect/indvars_1.ll
>
>Instead of pointing to some test cases, it would be good to explain the
>reason for the error.
>
>The problem is that we can currently can only model induction variables
>that are either fully part of our scop (-> we model them as loop
>induction variable) or induction variables that are scop-invariant (->
>we model them as parameter). A loop that is only partially part of the
>scop causes troubles, as there is no good way to handle the induction
>variable in the independent blocks pass.
>
if we just want to ensure that no scop region partially contains a loop, then
a better way is to check all region entering edges.
I have attached another patch file for this solution. In this patch file, we check
that if the region header is part of a loop, then no entering edges are come
from outside the region but within the loop. In such as case, I think we can
be sure that no scop region partially contains a loop.
>> ---
>> lib/Analysis/ScopDetection.cpp | 10 +++++++++
>> test/IndependentBlocks/indvars.ll | 3 +--
>> test/ScopDetect/indvars_1.ll | 43 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 54 insertions(+), 2 deletions(-)
>> create mode 100644 test/ScopDetect/indvars_1.ll
>>
>> diff --git a/lib/Analysis/ScopDetection.cpp b/lib/Analysis/ScopDetection.cpp
>> index 343ac83..edd07bf 100644
>> --- a/lib/Analysis/ScopDetection.cpp
>> +++ b/lib/Analysis/ScopDetection.cpp
>> @@ -138,6 +138,7 @@ STATISTIC(ValidRegion, "Number of regions that a valid part of Scop");
>>
>> BADSCOP_STAT(CFG, "CFG too complex");
>> BADSCOP_STAT(IndVar, "Non canonical induction variable in loop");
>> +BADSCOP_STAT(IndLoop, "Region only contains part of the loop");
>> BADSCOP_STAT(LoopBound, "Loop bounds can not be computed");
>> BADSCOP_STAT(FuncCall, "Function call with side effects appeared");
>> BADSCOP_STAT(AffFunc, "Expression not affine");
>> @@ -413,6 +414,15 @@ bool ScopDetection::isValidLoop(Loop *L, DetectionContext &Context) const {
>> if (!IndVar)
>> INVALID(IndVar,
>> "No canonical IV at loop header: " << L->getHeader()->getName());
>> + } else {
>> + // If code generation is in scev based mode, we need to ensure that
>> + // the region cannot contain only part of the loop.
>
>I don't think there is a need to restrict this to the scev based mode.
>
>> + Region &R = Context.CurRegion;
>> + for (Loop::block_iterator I = L->block_begin(), E = L->block_end();
>> + I != E; ++I)
>> + if (!R.contains(*I))
>> + INVALID(IndLoop, "Region only contains part of the loop:"
>> + << L->getHeader()->getName());
>> }
>
>Going over every basic block is too much. It should be sufficient to
>check that all exiting basic blocks are part of the region. As we
>currently only allow a single exiting block, the check can be simplified
>even more.
I think checking all existing basic blocks are still expensive. Could you revise
my new patch file, which should be much simpler than this solution .
>
>> // Is the loop count affine?
>> diff --git a/test/IndependentBlocks/indvars.ll b/test/IndependentBlocks/indvars.ll
>> index aa096f4..6f506bd 100644
>> --- a/test/IndependentBlocks/indvars.ll
>> +++ b/test/IndependentBlocks/indvars.ll
>> @@ -1,5 +1,4 @@
>> -; RUN: opt %loadPolly -polly-independent -polly-codegen-scev %s | FileCheck %s
>> -; XFAIL: *
>> +; RUN: opt %loadPolly -polly-independent -polly-codegen-scev < %s -S | FileCheck %s
>> ;
>> ; Ensure that the independent block pass does not invalidate the induction
>> ; variable here.
>
>You can probably remove this file entirely. The test case is not
>interesting any more as the problematic SCoP is not even detected.
Yes, I removed this testcase.
>
>> diff --git a/test/ScopDetect/indvars_1.ll b/test/ScopDetect/indvars_1.ll
>> new file mode 100644
>> index 0000000..ead1e4d
>> --- /dev/null
>> +++ b/test/ScopDetect/indvars_1.ll
>> @@ -0,0 +1,43 @@
>> +; RUN: opt %loadPolly -polly-codegen-isl -polly-codegen-scev < %s -S | FileCheck %s
>> +; RUN: opt %loadPolly -analyze -polly-detect -polly-codegen-isl -polly-codegen-scev < %s | FileCheck -check-prefix=CHECK-SCEV %s
>> +;
>> +; Ensure that the independent block pass does not invalidate the induction
>> +; variable here.
>
>This comment is outdated.
New comment is added.
>
>> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
>> +target triple = "x86_64-unknown-linux-gnu"
>> +
>> +define void @main(i64* %A) nounwind {
>> +entry:
>> + br label %for.i
>> +
>> +for.i:
>> + %indvar.i = phi i64 [ 0, %entry ], [ %indvar.next.i, %for.i.backedge ]
>> + %indvar.next.i = add i64 %indvar.i, 1
>> + %scevgep = getelementptr i64* %A, i64 %indvar.i
>> + store i64 %indvar.i, i64* %scevgep, align 4
>> + br i1 true, label %for.j.preheader, label %for.j2
>> +
>> +for.j.preheader:
>> + br label %for.j
>> +
>> +for.j:
>> + %indvar.j = phi i64 [ %indvar.next.j, %for.j ], [ 0, %for.j.preheader ]
>> + %indvar.next.j = add i64 %indvar.j, 1
>> + %exitcond.j = icmp eq i64 %indvar.next.j, 10
>> + br i1 %exitcond.j, label %for.j2, label %for.j
>> +
>> +for.j2:
>> + fence seq_cst
>> + br label %for.i.backedge
>> +
>> +for.i.backedge:
>> + %exitcond.i = icmp eq i64 %indvar.next.i, 2048
>> + br i1 %exitcond.i, label %for.i, label %.end
>> +
>> +.end:
>> + ret void
>> +}
>> +
>> +; CHECK: %indvar.j = phi i64 [ %indvar.next.j, %for.j ], [ 0, %polly.split_new_and_old ]
>> +; CHECK-SCEV: Valid Region for Scop: for.j => for.j2
>
>You should check for the output of -polly-detect (See the other test
>cases on the way).
>
>Tobi
>
Star Tan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130610/d00d3cb0/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ScopDetect-check-region-entering-edges-are-valid.patch
Type: application/octet-stream
Size: 5599 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130610/d00d3cb0/attachment.obj>
More information about the llvm-commits
mailing list