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 ();