| Message ID | 5124ED5F.5050302@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/20/2013 07:35 AM, Aldy Hernandez wrote: > In the following test, the first statement of a relaxed transaction is > an inline asm: > > __transaction_relaxed { __asm__(""); } > > Since we bypass inserting BUILT_IN_TM_IRREVOCABLE at the beginning of > transactions that are sure to be irrevocable, later when we try to > expand the transaction, we ICE when we encounter the inline asm. > > Currently, we bypass the TM_IRREVOCABLE call here: > > for (region = d->all_tm_regions; region; region = region->next) > { > /* If we're sure to go irrevocable, don't transform anything. */ > if (d->irrevocable_blocks_normal > && bitmap_bit_p (d->irrevocable_blocks_normal, > region->entry_block->index)) > { > transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE); > transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE); > continue; > } > > If I understand this correctly, ideally a transaction marked as > doesGoIrrevocable shouldn't bother instrumenting the statements inside, > since the runtime will go irrevocable immediately. In which case, we > can elide the instrumentation altogether as the attached patch does. > > If my analysis is correct, then testsuite/gcc.dg/tm/memopt-1.c would > surely go irrevocable, thus requiring no instrumentation, causing the > memory optimizations to get skipped altogether. In which case, it's > best to mark the function calls as safe, so they don't cause the > transaction to become obviously irrevocable. > > Is this correct? If so, OK? Yes, that's correct. The patch is ok. r~
diff --git a/gcc/testsuite/gcc.dg/tm/memopt-1.c b/gcc/testsuite/gcc.dg/tm/memopt-1.c index b78a6d4..c5ac5ce 100644 --- a/gcc/testsuite/gcc.dg/tm/memopt-1.c +++ b/gcc/testsuite/gcc.dg/tm/memopt-1.c @@ -2,8 +2,8 @@ /* { dg-options "-fgnu-tm -O -fdump-tree-tmmemopt" } */ long g, xxx, yyy; -extern george() __attribute__((transaction_callable)); -extern ringo(long int); +extern george() __attribute__((transaction_safe)); +extern ringo(long int) __attribute__((transaction_safe)); int i; f() diff --git a/gcc/testsuite/gcc.dg/tm/pr56108.c b/gcc/testsuite/gcc.dg/tm/pr56108.c new file mode 100644 index 0000000..81ff574 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tm/pr56108.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -O" } */ + +int +main() +{ + __transaction_relaxed { __asm__(""); } + return 0; +} diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c index dd3918e..71eaa44 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -2859,8 +2859,23 @@ execute_tm_mark (void) // Expand memory operations into calls into the runtime. // This collects log entries as well. FOR_EACH_VEC_ELT (bb_regions, i, r) - if (r != NULL) - expand_block_tm (r, BASIC_BLOCK (i)); + { + if (r != NULL) + { + if (r->transaction_stmt) + { + unsigned sub = gimple_transaction_subcode (r->transaction_stmt); + + /* If we're sure to go irrevocable, there won't be + anything to expand, since the run-time will go + irrevocable right away. */ + if (sub & GTMA_DOES_GO_IRREVOCABLE + && sub & GTMA_MAY_ENTER_IRREVOCABLE) + continue; + } + expand_block_tm (r, BASIC_BLOCK (i)); + } + } bb_regions.release ();
In the following test, the first statement of a relaxed transaction is an inline asm: __transaction_relaxed { __asm__(""); } Since we bypass inserting BUILT_IN_TM_IRREVOCABLE at the beginning of transactions that are sure to be irrevocable, later when we try to expand the transaction, we ICE when we encounter the inline asm. Currently, we bypass the TM_IRREVOCABLE call here: for (region = d->all_tm_regions; region; region = region->next) { /* If we're sure to go irrevocable, don't transform anything. */ if (d->irrevocable_blocks_normal && bitmap_bit_p (d->irrevocable_blocks_normal, region->entry_block->index)) { transaction_subcode_ior (region, GTMA_DOES_GO_IRREVOCABLE); transaction_subcode_ior (region, GTMA_MAY_ENTER_IRREVOCABLE); continue; } If I understand this correctly, ideally a transaction marked as doesGoIrrevocable shouldn't bother instrumenting the statements inside, since the runtime will go irrevocable immediately. In which case, we can elide the instrumentation altogether as the attached patch does. If my analysis is correct, then testsuite/gcc.dg/tm/memopt-1.c would surely go irrevocable, thus requiring no instrumentation, causing the memory optimizations to get skipped altogether. In which case, it's best to mark the function calls as safe, so they don't cause the transaction to become obviously irrevocable. Is this correct? If so, OK? PR middle-end/56108 * trans-mem.c (execute_tm_mark): Do not expand transactions that are sure to go irrevocable. testsuite/ * gcc.dg/tm/memopt-1.c: Declare functions transaction_safe.