| Message ID | 4D2CAFEB.2020607@redhat.com |
|---|---|
| State | New |
| Headers | show |
> /* Handle the easy initialization to zero. */ > - if (CONSTRUCTOR_ELTS (rhs) == 0) > + if (CONSTRUCTOR_ELTS (rhs) == 0 && INTEGRAL_TYPE_P (type)) > rhs = build_int_cst (type, 0); While this works, it's not what I intended. In particular, you'll never see a CONSTRUCTOR for an integral type, so this condition will never fire. What's intended here is to store a zero of the proper size into the location. So actually the little change is rhs = build_int_cst (simple_type, 0); This should probably be done as a separate fix, since it turns out to be non-obvious. > + /* If we saw something that will make us go irrevocable, put it > + in the worklist so we can scan the function later > + (ipa_tm_scan_irr_function) and mark the irrevocable > + blocks. */ > + if (node->local.tm_may_enter_irr) > + maybe_push_queue (node, &worklist, &d->in_worklist); Ok. You're probably right that the ipa pass needs to be rewritten. Third time's the charm, right? r~
Index: testsuite/c-c++-common/tm/inline-asm-2.c =================================================================== --- testsuite/c-c++-common/tm/inline-asm-2.c (revision 0) +++ testsuite/c-c++-common/tm/inline-asm-2.c (revision 0) @@ -0,0 +1,8 @@ +// { dg-do compile } +// { dg-options "-fgnu-tm" } + +__attribute__((transaction_callable)) +void func() +{ + __asm__ (""); +} Index: trans-mem.c =================================================================== --- trans-mem.c (revision 168675) +++ trans-mem.c (working copy) @@ -2067,7 +2067,7 @@ build_tm_store (location_t loc, tree lhs if (TREE_CODE (rhs) == CONSTRUCTOR) { /* Handle the easy initialization to zero. */ - if (CONSTRUCTOR_ELTS (rhs) == 0) + if (CONSTRUCTOR_ELTS (rhs) == 0 && INTEGRAL_TYPE_P (type)) rhs = build_int_cst (type, 0); else { @@ -4498,6 +4498,13 @@ ipa_tm_execute (void) a = cgraph_function_body_availability (node); d = get_cg_data (node); + /* If we saw something that will make us go irrevocable, put it + in the worklist so we can scan the function later + (ipa_tm_scan_irr_function) and mark the irrevocable + blocks. */ + if (node->local.tm_may_enter_irr) + maybe_push_queue (node, &worklist, &d->in_worklist); + /* Some callees cannot be arbitrarily cloned. These will always be irrevocable. Mark these now, so that we need not scan them. */ if (is_tm_irrevocable (node->decl))
One day when I'm bored, I'm going to rewrite this IPA TM pass. It's an ever present source of grief in my life. But alas, not today... Today's pain begins here: __attribute__((transaction_callable)) void func() { __asm__ (""); } We scan the function in diagnose_tm() and set the tm_may_enter_irr bit in the cgraph node. However, in the TM IPA pass, we don't mark irrevocable basic blocks (in ipa_tm_scan_irr_function()) unless the function is already in the (irrevocable) worklist. This patch adds functions marked with tm_may_enter_irr into the worklist, so we can later pick them up and scan their basic blocks for irrevocability, thus going irrevocable at the correct place. There is also a gratuitous fix to instrumentation of constructors. We currently ICE on aggregate types. All this is actually background for an upcoming one liner which has uncovered all sorts of pain throughout the TM engine. OK for branch? * trans-mem.c (ipa_tm_execute): Place possibly irrevocable functions in worklist. (build_tm_store): Only call build_int_cst with integers.