[PATCH][Polly] Re: 0001-Fixup-assert-fails-caused-by-incorrect-LoopInfo-update

[Star Tan]

At 2013-08-25 02:10:02,"Tobias Grosser" <tobias at grosser.es> wrote:
>On 08/23/2013 08:25 AM, Star Tan wrote:
>> Hi all,
>>
>>
>> I have attached a patch file to fix up the CodeGen assert fail, which is also reported in a recent bug  http://llvm.org/bugs/show_bug.cgi?id=16944
>
>Hi Star Tan,
>
>thanks for looking into this bug. The patch itself looks good and the 
>testcase is nicely small.
>
>However, when running the test with 'opt' I get:
>
>PHINode should have one entry for each predecessor of its parent basic 
>block!
>   %indvar = phi i64 [ 0, %entry ], [ %0, %for.inc ]
>
>So the IR in the test case itself is already incorrect.

Sorry for the stupid mistake. I have attached a new patch file!>Also, I am a little unhappy with the fact that we need to run 

>-polly-opt-isl to reproduce this bug. I would very much prefer to find a 
>test case that only runs 'opt %loadPolly -polly-codegen  < %s'. This ensures
>that changes in -polly-opt-isl can not accidentally stop this test case 
>from testing the code path you fixed in this patch.

Yes, you are right. But it seems difficult to construct a testcase that will show this bug using only "opt %loadPolly -polly-codegen".
Our testcase fails with the following command:
$ opt %loadPolly -basicaa -polly-opt-isl -polly-codegen test.ll

but it works well with the following commands:
$ opt %loadPolly -basicaa -polly-opt-isl -S -o test_1.ll test.ll
$ opt %loadPolly -basicaa -polly-codegen test_1.ll

The key of producing the bug is to construct a stmt_guard statment, so it will call codegen((clast_guard *)). Preliminary investigation shows that the  'stmt_guard' occurs only if we run opt-isl before codegen.  Sorry I have not found such a testcase by now.

Can anyone provide some hints about the stmt_guard as used in lib/CodeGen/CodeGeneration.cpp:955
  if (CLAST_STMT_IS_A(stmt, stmt_root))
    assert(false && "No second root statement expected");
  else if (CLAST_STMT_IS_A(stmt, stmt_ass))
    codegen((const clast_assignment *)stmt);
  else if (CLAST_STMT_IS_A(stmt, stmt_user))
    codegen((const clast_user_stmt *)stmt);
  else if (CLAST_STMT_IS_A(stmt, stmt_block))
    codegen((const clast_block *)stmt);
  else if (CLAST_STMT_IS_A(stmt, stmt_for))
    codegen((const clast_for *)stmt);
  else if (CLAST_STMT_IS_A(stmt, stmt_guard))
    codegen((const clast_guard *)stmt);    // this is the branch we want to go.

>> The assert fail is caused by those LoopInfo updates added in r181987.
>> Note that the LoopInfo of MergeBlock has been updated in the function "SplitBlock", so it should not be updated again in CodeGen.
>>

Star Tan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130902/d8b924f9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fixup-assert-fails-caused-by-incorrect-LoopInfo-upda.patch
Type: application/octet-stream
Size: 2461 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130902/d8b924f9/attachment.obj>