Hal, I think "declared here" messages should point to the real
declaration of the variable. "firstprivate" is an implicit declaration.
Besides, if the original variable is not used in the capturedstmt
construct it won't be captured at all and I won't be able to use its
value in the OpenMP construct for initialization
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
25.09.2014 7:23, Hal Finkel пишет:
----- Original Message -----
From: "Alexey Bataev" <[email protected]>
To: "Hal Finkel" <[email protected]>
Cc: [email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected]
Sent: Wednesday, September 24, 2014 9:45:56 PM
Subject: Re: [PATCH] [OPENMP] Codegen for 'firstprivate' clause in 'parallel'
directive
It does not look like you call Sema::PushOnScopeChains with the new
private variable declaration. Why?
Because this variable is not used in Sema, so the resolver does not
need
to look for this private variable. Actual replacing of the original
variable by the private one occurs in codegen.
I figured as much, but why is that the design you've chosen? Would it be better
to have Sema use that variable? If nothing else, it might yield better
'declared here' notes on errors/warnings (or worse ones, depending on whether
or not you want the 'declared here' notes to point to the firstprivate/private
clauses or not).
-Hal
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
24.09.2014 18:09, Hal Finkel пишет:
----- Original Message -----
From: "Alexey Bataev" <[email protected]>
To: "Hal Finkel" <[email protected]>
Cc: [email protected], [email protected],
[email protected], [email protected],
[email protected],
[email protected],
[email protected]
Sent: Wednesday, September 24, 2014 2:34:53 AM
Subject: Re: [PATCH] [OPENMP] Codegen for 'firstprivate' clause in
'parallel' directive
Hal, All these changes are part of codegen. I replaced our temp
checks
for firstprivate vars to the proper one.
Private copies are required for correct codegen of firstprivate
vars
with constructors/destructors and for correct debug info (if you
try
to
debug code generated by clang from clang-omp.github.io you will
have
a
lot of troubles with it, because there is no correct debug info
for
private vars
Yes, I've noticed this ;)
).
For each firstprivate var we're generating a private copy inside
an
OpenMP region that will be used instead of the original var. Then
we're
generating an intializer for these private vars with the value of
the
corresponding original variable. Also we need some additional
helper
variables if the firstprivate variable has an array type. In this
case
we're generating initializer for a single item of that array and
in
codegen use this initializer for proper array initialization.
To some extent, I understand. firstprivate(i) is semantically like
a declaration of a new variable i that is initialized with the old
variable i. It does seem similar to what is done for
explicitly-named lambda capture variables (in
Sema::ActOnStartOfLambdaDefinition where it calls
Sema::createLambdaInitCaptureVarDecl). It does not look like you
call Sema::PushOnScopeChains with the new private variable
declaration. Why?
Thanks again,
Hal
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
23.09.2014 16:56, Hal Finkel пишет:
----- Original Message -----
From: "Alexey Bataev" <[email protected]>
To: "a bataev" <[email protected]>, [email protected],
[email protected], [email protected],
[email protected], [email protected], [email protected]
Cc: [email protected]
Sent: Monday, September 22, 2014 11:26:56 PM
Subject: Re: [PATCH] [OPENMP] Codegen for 'firstprivate' clause
in
'parallel' directive
Please separate out the Sema changes from this patch into a
separate patch.
Hal, the changes in Sema are required for codegen only. I won't
be
able to test them without codegen.
Okay, obviously there are changes to the Sema tests because the
messages have changed. Can you commit that change separately? If
this is not reasonable/possible, please explain why.
Also, regarding the addition of the '*PrivateCopies' functions to
the AST nodes, can you please explain the design? I don't
understand why you seem to be duplicating some of the variables
in
the AST.
Thanks again,
Hal
[snip]
http://reviews.llvm.org/D5140
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
