[PATCH] [OPENMP] Codegen for 'firstprivate' clause in 'parallel' directive

[Hal Finkel]

----- Original Message -----
> From: "Alexey Bataev" <a.bataev at hotmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: cfe-commits at cs.uiuc.edu, dgregor at apple.com, fraggamuffin at gmail.com, richard at metafoo.co.uk, rjmccall at gmail.com,
> ejstotzer at gmail.com, reviews+d5140+public+cb73bc52a9cc08f9 at reviews.llvm.org
> 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" <a.bataev at hotmail.com>
> >> To: "a bataev" <a.bataev at hotmail.com>, dgregor at apple.com,
> >> fraggamuffin at gmail.com, richard at metafoo.co.uk,
> >> rjmccall at gmail.com, ejstotzer at gmail.com, hfinkel at anl.gov
> >> Cc: cfe-commits at cs.uiuc.edu
> >> 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
> >>
> >>
> >>
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory