* Re: [trans-mem] issue with openmp [not found] <4C04C24A.4080103@unine.ch> @ 2010-06-18 16:47 ` Aldy Hernandez 2010-06-22 18:30 ` Aldy Hernandez [not found] ` <20100618153140.GB7343@redhat.com> 1 sibling, 1 reply; 9+ messages in thread From: Aldy Hernandez @ 2010-06-18 16:47 UTC (permalink / raw) To: Patrick Marlier Cc: Richard Henderson, FELBER Pascal, Javier Arias, gcc-patches The problem here is that gimplify_transaction() places the temporaries that were generated for a transaction in cfun->local_decls, but omp_copy_decl() will only look in the enclosing contexts, not in cfun->local_decls. rth suggested we make a better attempt at putting temporaries into the proper context so OMP can figure out how to pull pieces out to make a new function. The patch below wraps the transaction bodies into a BIND_EXPR, which gimplify_transaction() can later use for its temporaries, thus allowing the OMP code to find a proper context. OK for branch? * c-typeck.c (c_finish_transaction): Same. * cp/semantics.c (finish_transaction_stmt): Wrap transaction body in a BIND_EXPR. Index: testsuite/c-c++-common/tm/omp.c =================================================================== --- testsuite/c-c++-common/tm/omp.c (revision 0) +++ testsuite/c-c++-common/tm/omp.c (revision 0) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -fopenmp" } */ + +__attribute__ ((transaction_pure)) +unsigned long rdtsc(); + +typedef struct ENTER_EXIT_TIMES +{ + unsigned long enter; +} times_t; + +void ParClassify() +{ + void * Parent; +#pragma omp parallel private(Parent) + { + times_t inside; + __transaction [[atomic]] { + inside.enter = rdtsc(); + } + } +} Index: cp/semantics.c =================================================================== --- cp/semantics.c (revision 160538) +++ cp/semantics.c (working copy) @@ -4683,7 +4683,18 @@ begin_transaction_stmt (location_t loc, void finish_transaction_stmt (tree stmt, tree compound_stmt, int flags) { - TRANSACTION_EXPR_BODY (stmt) = pop_stmt_list (TRANSACTION_EXPR_BODY (stmt)); + tree body = pop_stmt_list (TRANSACTION_EXPR_BODY (stmt)); + + /* Wrap the transaction body in a BIND_EXPR so we have a context + where to put decls for OpenMP. */ + if (TREE_CODE (body) != BIND_EXPR) + { + body = build3 (BIND_EXPR, void_type_node, NULL, body, NULL); + TREE_SIDE_EFFECTS (body) = 1; + SET_EXPR_LOCATION (body, EXPR_LOCATION (stmt)); + } + + TRANSACTION_EXPR_BODY (stmt) = body; TRANSACTION_EXPR_OUTER (stmt) = (flags & TM_STMT_ATTR_OUTER) != 0; TRANSACTION_EXPR_RELAXED (stmt) = (flags & TM_STMT_ATTR_RELAXED) != 0; Index: c-typeck.c =================================================================== --- c-typeck.c (revision 160538) +++ c-typeck.c (working copy) @@ -10281,9 +10281,20 @@ c_finish_omp_clauses (tree clauses) /* Create a transaction node. */ tree -c_finish_transaction (location_t loc, tree block, int flags) +c_finish_transaction (location_t loc, tree body, int flags) { - tree stmt = build_stmt (loc, TRANSACTION_EXPR, block); + tree stmt; + + /* Wrap the transaction body in a BIND_EXPR so we have a context + where to put decls for OpenMP. */ + if (TREE_CODE (body) != BIND_EXPR) + { + body = build3 (BIND_EXPR, void_type_node, NULL, body, NULL); + TREE_SIDE_EFFECTS (body) = 1; + SET_EXPR_LOCATION (body, loc); + } + + stmt = build_stmt (loc, TRANSACTION_EXPR, body); if (flags & TM_STMT_ATTR_OUTER) TRANSACTION_EXPR_OUTER (stmt) = 1; if (flags & TM_STMT_ATTR_RELAXED) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-06-18 16:47 ` [trans-mem] issue with openmp Aldy Hernandez @ 2010-06-22 18:30 ` Aldy Hernandez 2010-06-22 19:00 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Aldy Hernandez @ 2010-06-22 18:30 UTC (permalink / raw) To: Patrick Marlier Cc: Richard Henderson, FELBER Pascal, Javier Arias, gcc-patches On Fri, Jun 18, 2010 at 11:26:00AM -0400, Aldy Hernandez wrote: > The problem here is that gimplify_transaction() places the temporaries > that were generated for a transaction in cfun->local_decls, but > omp_copy_decl() will only look in the enclosing contexts, not in > cfun->local_decls. > > rth suggested we make a better attempt at putting temporaries into the > proper context so OMP can figure out how to pull pieces out to make a > new function. > > The patch below wraps the transaction bodies into a BIND_EXPR, which > gimplify_transaction() can later use for its temporaries, thus allowing > the OMP code to find a proper context. > > OK for branch? Meanwhile, back at the ranch... rth complains that we should do this in the gimplifier and save the front-end work. Yay, less code! OK for branch? * gimplify.c (gimplify_transaction): Wrap transaction body in a BIND_EXPR. Index: testsuite/c-c++-common/tm/omp.c =================================================================== --- testsuite/c-c++-common/tm/omp.c (revision 0) +++ testsuite/c-c++-common/tm/omp.c (revision 0) @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -fopenmp" } */ + +__attribute__ ((transaction_pure)) +unsigned long rdtsc(); + +typedef struct ENTER_EXIT_TIMES +{ + unsigned long enter; +} times_t; + +void ParClassify() +{ + void * Parent; +#pragma omp parallel private(Parent) + { + times_t inside; + __transaction [[atomic]] { + inside.enter = rdtsc(); + } + } +} Index: gimplify.c =================================================================== --- gimplify.c (revision 161187) +++ gimplify.c (working copy) @@ -6385,20 +6385,27 @@ gimplify_omp_atomic (tree *expr_p, gimpl static enum gimplify_status gimplify_transaction (tree *expr_p, gimple_seq *pre_p) { - tree expr = *expr_p, temp; + tree expr = *expr_p, temp, tbody = TRANSACTION_EXPR_BODY (expr); gimple g; gimple_seq body = NULL; struct gimplify_ctx gctx; int subcode = 0; + /* Wrap the transaction body in a BIND_EXPR so we have a context + where to put decls for OpenMP. */ + if (TREE_CODE (tbody) != BIND_EXPR) + { + tree bind = build3 (BIND_EXPR, void_type_node, NULL, tbody, NULL); + TREE_SIDE_EFFECTS (bind) = 1; + SET_EXPR_LOCATION (bind, EXPR_LOCATION (tbody)); + TRANSACTION_EXPR_BODY (expr) = bind; + } + push_gimplify_context (&gctx); temp = voidify_wrapper_expr (*expr_p, NULL); g = gimplify_and_return_first (TRANSACTION_EXPR_BODY (expr), &body); - if (g && gimple_code (g) == GIMPLE_BIND) - pop_gimplify_context (g); - else - pop_gimplify_context (NULL); + pop_gimplify_context (g); g = gimple_build_transaction (body, NULL); if (TRANSACTION_EXPR_OUTER (expr)) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-06-22 18:30 ` Aldy Hernandez @ 2010-06-22 19:00 ` Richard Henderson 0 siblings, 0 replies; 9+ messages in thread From: Richard Henderson @ 2010-06-22 19:00 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Patrick Marlier, FELBER Pascal, Javier Arias, gcc-patches On 06/22/2010 11:17 AM, Aldy Hernandez wrote: > On Fri, Jun 18, 2010 at 11:26:00AM -0400, Aldy Hernandez wrote: >> The problem here is that gimplify_transaction() places the temporaries >> that were generated for a transaction in cfun->local_decls, but >> omp_copy_decl() will only look in the enclosing contexts, not in >> cfun->local_decls. >> >> rth suggested we make a better attempt at putting temporaries into the >> proper context so OMP can figure out how to pull pieces out to make a >> new function. >> >> The patch below wraps the transaction bodies into a BIND_EXPR, which >> gimplify_transaction() can later use for its temporaries, thus allowing >> the OMP code to find a proper context. >> >> OK for branch? > > Meanwhile, back at the ranch... rth complains that we should do this in > the gimplifier and save the front-end work. > > Yay, less code! > > OK for branch? > > * gimplify.c (gimplify_transaction): Wrap transaction body > in a BIND_EXPR. Ok. r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20100618153140.GB7343@redhat.com>]
[parent not found: <4C221F08.2020403@unine.ch>]
[parent not found: <20100623172043.GB19259@redhat.com>]
[parent not found: <4C225BC3.9010301@unine.ch>]
* Re: [trans-mem] issue with openmp [not found] ` <4C225BC3.9010301@unine.ch> @ 2010-07-06 17:35 ` Aldy Hernandez 2010-07-06 17:48 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Aldy Hernandez @ 2010-07-06 17:35 UTC (permalink / raw) To: Patrick Marlier; +Cc: Richard Henderson, FELBER Pascal, gcc-patches > >>- libitm : I think it is useful to add _ITM_malloc, _ITM_free, > >>_ITM_calloc into libitm.h and also to add ITM_REGPARM because I had > >>problems when I wanted to use directly the library. Patrick found a problem while calling the _ITM_malloc/etc functions directly (instead of through the wrappers). I've added prototypes for them. OK for branch? Patrick, please verify that this fixes any problems on your end. * libitm.h (ITM_PURE): Define. Declare _ITM_malloc, _ITM_calloc, and _ITM_free. Index: libitm.h =================================================================== --- libitm.h (revision 161512) +++ libitm.h (working copy) @@ -43,6 +43,7 @@ extern "C" { #endif #define ITM_NORETURN __attribute__((noreturn)) +#define ITM_PURE __attribute__((transaction_pure)) \f /* The following are externally visible definitions and functions, though only very few of these should be called by user code. */ @@ -152,8 +153,13 @@ extern void _ITM_addUserUndoAction(_ITM_ extern int _ITM_getThreadnum(void) ITM_REGPARM; -__attribute__((transaction_pure)) -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; + +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE +extern void *_ITM_malloc (size_t); +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE +extern void *_ITM_calloc (size_t, size_t); +extern void _ITM_free (void *) ITM_REGPARM ITM_PURE; /* The following typedefs exist to make the macro expansions below work ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-07-06 17:35 ` Aldy Hernandez @ 2010-07-06 17:48 ` Richard Henderson 2010-07-06 18:12 ` Aldy Hernandez 0 siblings, 1 reply; 9+ messages in thread From: Richard Henderson @ 2010-07-06 17:48 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Patrick Marlier, FELBER Pascal, gcc-patches On 07/06/2010 09:29 AM, Aldy Hernandez wrote: > -__attribute__((transaction_pure)) > -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; > +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; > + > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE > +extern void *_ITM_malloc (size_t); > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE > +extern void *_ITM_calloc (size_t, size_t); > +extern void _ITM_free (void *) ITM_REGPARM ITM_PURE; I'm not sure that _ITM_malloc et al should include ITM_REGPARM. Also, it's canonical to place these attributes before the ";", not before the "extern". r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-07-06 17:48 ` Richard Henderson @ 2010-07-06 18:12 ` Aldy Hernandez 2010-07-06 19:28 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Aldy Hernandez @ 2010-07-06 18:12 UTC (permalink / raw) To: Richard Henderson; +Cc: Patrick Marlier, FELBER Pascal, gcc-patches On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote: > On 07/06/2010 09:29 AM, Aldy Hernandez wrote: > > -__attribute__((transaction_pure)) > > -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; > > +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; > > + > > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE > > +extern void *_ITM_malloc (size_t); > > +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE > > +extern void *_ITM_calloc (size_t, size_t); > > +extern void _ITM_free (void *) ITM_REGPARM ITM_PURE; > > I'm not sure that _ITM_malloc et al should include ITM_REGPARM. Arghh, I was getting confused by a complaint by Patrick about a %eax<->%rax problem in the calling sequence, but that must be something different because I can't reproduce it. Direct calls to _ITM_malloc() agree with the calling convention expected by such function. Patrick, if after a complete toolchain rebuild with this patch, you still see an ABI problem, send me the testcase. > Also, it's canonical to place these attributes before the ";", > not before the "extern". Fixed. OK? * libitm.h (ITM_PURE): Define. Declare _ITM_malloc, _ITM_calloc, and _ITM_free. Index: libitm.h =================================================================== --- libitm.h (revision 161512) +++ libitm.h (working copy) @@ -43,6 +43,7 @@ extern "C" { #endif #define ITM_NORETURN __attribute__((noreturn)) +#define ITM_PURE __attribute__((transaction_pure)) \f /* The following are externally visible definitions and functions, though only very few of these should be called by user code. */ @@ -152,8 +153,15 @@ extern void _ITM_addUserUndoAction(_ITM_ extern int _ITM_getThreadnum(void) ITM_REGPARM; -__attribute__((transaction_pure)) -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; + +extern void *_ITM_malloc (size_t) + __attribute__((__malloc__)) ITM_PURE; + +extern void *_ITM_calloc (size_t, size_t) + __attribute__((__malloc__)) ITM_PURE; + +extern void _ITM_free (void *) ITM_PURE; /* The following typedefs exist to make the macro expansions below work ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-07-06 18:12 ` Aldy Hernandez @ 2010-07-06 19:28 ` Richard Henderson 2010-07-06 20:58 ` Patrick Marlier 0 siblings, 1 reply; 9+ messages in thread From: Richard Henderson @ 2010-07-06 19:28 UTC (permalink / raw) To: Aldy Hernandez; +Cc: Patrick Marlier, FELBER Pascal, gcc-patches On 07/06/2010 11:12 AM, Aldy Hernandez wrote: > On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote: >> On 07/06/2010 09:29 AM, Aldy Hernandez wrote: >>> -__attribute__((transaction_pure)) >>> -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; >>> +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; >>> + >>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE >>> +extern void *_ITM_malloc (size_t); >>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE >>> +extern void *_ITM_calloc (size_t, size_t); >>> +extern void _ITM_free (void *) ITM_REGPARM ITM_PURE; >> >> I'm not sure that _ITM_malloc et al should include ITM_REGPARM. > > Arghh, I was getting confused by a complaint by Patrick about a > %eax<->%rax problem in the calling sequence, but that must be something > different because I can't reproduce it. That was due to C's implicit type of "int" for undeclared functions. > * libitm.h (ITM_PURE): Define. > Declare _ITM_malloc, _ITM_calloc, and _ITM_free. Ok. r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-07-06 19:28 ` Richard Henderson @ 2010-07-06 20:58 ` Patrick Marlier 2010-07-06 23:26 ` Richard Henderson 0 siblings, 1 reply; 9+ messages in thread From: Patrick Marlier @ 2010-07-06 20:58 UTC (permalink / raw) To: Richard Henderson; +Cc: Aldy Hernandez, FELBER Pascal, gcc-patches On 07/06/2010 09:28 PM, Richard Henderson wrote: > On 07/06/2010 11:12 AM, Aldy Hernandez wrote: >> On Tue, Jul 06, 2010 at 10:48:20AM -0700, Richard Henderson wrote: >>> On 07/06/2010 09:29 AM, Aldy Hernandez wrote: >>>> -__attribute__((transaction_pure)) >>>> -extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM; >>>> +extern void _ITM_dropReferences (void *, size_t) ITM_REGPARM ITM_PURE; >>>> + >>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE >>>> +extern void *_ITM_malloc (size_t); >>>> +__attribute__((__malloc__)) ITM_REGPARM ITM_PURE >>>> +extern void *_ITM_calloc (size_t, size_t); >>>> +extern void _ITM_free (void *) ITM_REGPARM ITM_PURE; >>> >>> I'm not sure that _ITM_malloc et al should include ITM_REGPARM. Can you explain me why _ITM_malloc shouldn't have ITM_REGPARM whereas for example _ITM_memsetW must have it? >> Arghh, I was getting confused by a complaint by Patrick about a >> %eax<->%rax problem in the calling sequence, but that must be something >> different because I can't reproduce it. > > That was due to C's implicit type of "int" for undeclared functions. > >> * libitm.h (ITM_PURE): Define. >> Declare _ITM_malloc, _ITM_calloc, and _ITM_free. Yes it solved my issue. Thank you! Patrick. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [trans-mem] issue with openmp 2010-07-06 20:58 ` Patrick Marlier @ 2010-07-06 23:26 ` Richard Henderson 0 siblings, 0 replies; 9+ messages in thread From: Richard Henderson @ 2010-07-06 23:26 UTC (permalink / raw) To: Patrick Marlier; +Cc: Aldy Hernandez, FELBER Pascal, gcc-patches On 07/06/2010 01:58 PM, Patrick Marlier wrote: > Can you explain me why _ITM_malloc shouldn't have ITM_REGPARM whereas > for example _ITM_memsetW must have it? The other functions in that header are defined as part of the Intel ABI, and that ABI specifies REGPARM. _ITM_malloc is not part of that ABI. r~ ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-07-06 23:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <4C04C24A.4080103@unine.ch>
2010-06-18 16:47 ` [trans-mem] issue with openmp Aldy Hernandez
2010-06-22 18:30 ` Aldy Hernandez
2010-06-22 19:00 ` Richard Henderson
[not found] ` <20100618153140.GB7343@redhat.com>
[not found] ` <4C221F08.2020403@unine.ch>
[not found] ` <20100623172043.GB19259@redhat.com>
[not found] ` <4C225BC3.9010301@unine.ch>
2010-07-06 17:35 ` Aldy Hernandez
2010-07-06 17:48 ` Richard Henderson
2010-07-06 18:12 ` Aldy Hernandez
2010-07-06 19:28 ` Richard Henderson
2010-07-06 20:58 ` Patrick Marlier
2010-07-06 23:26 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).