----- 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 10:58:49 PM > Subject: Re: [PATCH] [OPENMP] Codegen for 'firstprivate' clause in 'parallel' > directive > > 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
Okay, good to know. Please encode this information in a comment somewhere. -Hal > > 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 > >>>>>> > >>>>>> > >>>>>> > >> > > -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
