[OMP] Clang fails to handle parallel for + collapse

Hi there,

It seems like Clang fails to create a OMPParallelForDirective object for the given GCC test sample: gcc/loop-9.c at master · gcc-mirror/gcc · GitHub

I just tried to dump the AST with clang cc1 -ast-dump -fopenmp loop-9.c, and the output is:

TranslationUnitDecl 0x3a53eb0 <<invalid sloc>> <invalid sloc>

-TypedefDecl 0x3a543c8 <<invalid sloc>> <invalid sloc> implicit

__int128_t '__int128'

`-BuiltinType 0x3a54100 '__int128'
-TypedefDecl 0x3a54428 <<invalid sloc>> <invalid sloc> implicit

__uint128_t 'unsigned __int128'

`-BuiltinType 0x3a54120 'unsigned __int128'
-TypedefDecl 0x3a546e8 <<invalid sloc>> <invalid sloc> implicit

__NSConstantString 'struct __NSConstantString_tag'

`-RecordType 0x3a54500 'struct __NSConstantString_tag'
  `-Record 0x3a54478 '__NSConstantString_tag'
-TypedefDecl 0x3a54778 <<invalid sloc>> <invalid sloc> implicit

__builtin_ms_va_list 'char *'

`-PointerType 0x3a54740 'char *'
  `-BuiltinType 0x3a53f40 'char'
-TypedefDecl 0x3a54a28 <<invalid sloc>> <invalid sloc> implicit

__builtin_va_list 'struct __va_list_tag [1]'

`-ConstantArrayType 0x3a549d0 'struct __va_list_tag [1]' 1
  `-RecordType 0x3a54850 'struct __va_list_tag'
    `-Record 0x3a547c8 '__va_list_tag'
-FunctionDecl 0x3aa0780 <tests/gcc-testsuite/libgomp.c/loop-9.c:1:13>

col:13 implicit abort 'void (void) __attribute__((noreturn))' extern

-FunctionDecl 0x3aa0828 prev 0x3aa0780 <col:1, col:24> col:13 used

abort 'void (void) __attribute__((noreturn))' extern

-VarDecl 0x3aa0948 <line:3:1, col:15> col:6 used buf 'char [8]' cinit
`-StringLiteral 0x3aa0a28 <col:15> 'char [8]' lvalue "01234567"
-VarDecl 0x3aa0a98 <line:4:1, col:16> col:6 used buf2 'char [8]' cinit
`-StringLiteral 0x3aa0af8 <col:16> 'char [8]' lvalue "23456789"

`-FunctionDecl 0x3aa0bf0 <line:6:1, line:18:1> line:7:1 main 'int (void)'
   `-CompoundStmt 0x3aa4fe0 <line:8:1, line:18:1>
     >-DeclStmt 0x3aa0dc0 <line:9:3, col:14>
     > >-VarDecl 0x3aa0cd8 <col:3, col:9> col:9 used p 'char *'
     > `-VarDecl 0x3aa0d48 <col:3, col:13> col:13 used q 'char *'
     >-DeclStmt 0x3aa0e68 <line:10:3, col:14>
     > `-VarDecl 0x3aa0de8 <col:3, col:13> col:7 used sum 'int' cinit
     > `-IntegerLiteral 0x3aa0e48 <col:13> 'int' 0
     >-IfStmt 0x3aa4f78 <line:15:3, line:16:12>
     > >-<<<NULL>>>
     > >-BinaryOperator 0x3aa4e98 <line:15:7, col:47> 'int' '||'
     > > >-BinaryOperator 0x3aa4de8 <col:7, col:35> 'int' '||'
     > > > >-BinaryOperator 0x3aa4cd0 <col:7, col:18> 'int' '!='
     > > > > >-ImplicitCastExpr 0x3aa4cb8 <col:7> 'char *' <LValueToRValue>
     > > > > > `-DeclRefExpr 0x3aa4be8 <col:7> 'char *' lvalue Var 0x3aa0cd8 'p' 'char *'
     > > > > `-UnaryOperator 0x3aa4c98 <col:12, col:18> 'char *' prefix '&'
     > > > > `-ArraySubscriptExpr 0x3aa4c70 <col:13, col:18> 'char' lvalue
     > > > > >-ImplicitCastExpr 0x3aa4c58 <col:13> 'char *' <ArrayToPointerDecay>
     > > > > > `-DeclRefExpr 0x3aa4c10 <col:13> 'char [8]' lvalue Var 0x3aa0948 'buf' 'char [8]'
     > > > > `-IntegerLiteral 0x3aa4c38 <col:17> 'int' 8
     > > > `-BinaryOperator 0x3aa4dc0 <col:23, col:35> 'int' '!='
     > > > >-ImplicitCastExpr 0x3aa4da8 <col:23> 'char *' <LValueToRValue>
     > > > > `-DeclRefExpr 0x3aa4cf8 <col:23> 'char *' lvalue Var 0x3aa0d48 'q' 'char *'
     > > > `-BinaryOperator 0x3aa4d80 <col:28, col:35> 'char *' '+'
     > > > >-ImplicitCastExpr 0x3aa4d68 <col:28> 'char *' <ArrayToPointerDecay>
     > > > > `-DeclRefExpr 0x3aa4d20 <col:28> 'char [8]' lvalue Var 0x3aa0a98 'buf2' 'char [8]'
     > > > `-IntegerLiteral 0x3aa4d48 <col:35> 'int' 8
     > > `-BinaryOperator 0x3aa4e70 <col:40, col:47> 'int' '!='
     > > >-ImplicitCastExpr 0x3aa4e58 <col:40> 'int' <LValueToRValue>
     > > > `-DeclRefExpr 0x3aa4e10 <col:40> 'int' lvalue Var 0x3aa0de8 'sum' 'int'
     > > `-IntegerLiteral 0x3aa4e38 <col:47> 'int' 576
     > >-CallExpr 0x3aa4f50 <line:16:5, col:12> 'void'
     > > `-ImplicitCastExpr 0x3aa4f38 <col:5> 'void (*)(void) __attribute__((noreturn))' <FunctionToPointerDecay>
     > > `-DeclRefExpr 0x3aa4ee0 <col:5> 'void (void) __attribute__((noreturn))' Function 0x3aa0828 'abort' 'void (void) __attribute__((noreturn))'
     > `-<<<NULL>>>
     `-ReturnStmt 0x3aa4fc8 <line:17:3, col:10>
       `-IntegerLiteral 0x3aa4fa8 <col:10> 'int' 0

As you can see, there is no OMPParallelForDirective.

Is it a bug in Clang? Or maybe the OMPParallelForDirective is not created because the loop doesn't respect the canonical form described by the OpenMP spec? In this case, it should be much better to return an error message like "for is not canonical" or something.

This test can be compiled with GCC 5.3.

Thanks!

Hi,
Thanks for the report. Problem fixed in r261080

Best regards,
Alexey Bataev

Hi Alexey,

I think this reveals another bug: q isn't correctly lastprivate and in the end doesn't hold buf2 + 8.
This may be related to collapse(2) because it works fine if I remove that clause...

Thanks,
Jonas

Seems to me we've run into some corner case. I'll fix this soon

Best regards,
Alexey Bataev

Seems to me we've run into some corner case. I'll fix this soon

I didn't see any regressions with my other OpenMP tests and this loop-9.c is fixed now, but I believe you :-). Thanks.

Yes, there is a bug. If you'll try to execute the test it will fail.
Patch will fix it and will produce a little bit faster code for
lastprivate loop counters

Best regards,
Alexey Bataev

Fixed in r261209

Best regards,
Alexey Bataev

Hi Alexey,

Have all of the recent OpenMP fixes made it into the release branch. Should this one?

-Hal

Hal, yes, this one also must be committed to release branch. Could you
ask Hans to commit it?

Best regards,
Alexey Bataev

Hans?

Thanks again,
Hal

r261080 doesn't merge cleanly. I get this conflict in the end of
BuildCounterUpdate:

<<<<<<< .working
  Update = SemaRef.BuildBinOp(S, Loc, BO_Assign, VarRef.get(), Update.get());

Hans, keep the code from ".working" section to resolve the conflict

Best regards,
Alexey Bataev

r261080 merged in r261256; Alexey, please check that I got the merge right.
r261209 merged in r261257.

Thanks,
Hans

Hans, thanks, everything seems ok

Best regards,
Alexey Bataev