http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52173
Aldy Hernandez <aldyh at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |amacleod at redhat dot com,
| |dnovillo at gcc dot
| |gnu.org, rguenth at gcc dot
| |gnu.org
--- Comment #9 from Aldy Hernandez <aldyh at gcc dot gnu.org> 2012-09-19
19:44:14 UTC ---
Richi, Diego. Perhaps you can shed light on this.
Here we have a loop unroll that triggers a "virtual def operand missing..." for
a GIMPLE_TRANSACTION:
int vec[500];
void func()
{
__transaction_relaxed
{
vec[123] = 456;
}
}
main()
{
int i;
for(i = 0; i < 10; ++i)
func();
}
The unroller wants to unroll into something like:
<bb 2>:
# .MEM_6 = VDEF <.MEM_13>
__transaction_relaxed <-- PROBLEMATIC INSN (VDEF's)
<bb 3>:
# .MEM_9 = VDEF <.MEM_59>
vec[123] = 456;
# .MEM_2 = VDEF <.MEM_9>
__builtin__ITM_commitTransaction ();
<bb 4>:
ivtmp_14 = 9;
# .MEM_11 = VDEF <.MEM_13>
__transaction_relaxed
<bb 5>:
# .MEM_18 = VDEF <.MEM_59>
vec[123] = 456;
# .MEM_19 = VDEF <.MEM_18>
__builtin__ITM_commitTransaction ();
etc etc etc.
Putting aside how incredibly inefficient this is... (We should be duplicating
the vector store, not the entire transaction)...
The problem here is that, after unrolling, the VDEF of the problematic insn is
.MEM_6, but the DEF_OP of the insn is .MEM_59. So gimple_vdef() !=
gimple_vdef_op() and we trigger a "virtual def operand missing for stmt".
The problem is that the unroller's call to gimple_duplicate_bb() will make a
copy of the problematic instruction, maintaining its gimple_vdef, but changing
all its def_ops from under us, thus making gimple_vdef() != gimple_vdef_op().
Before loop unrolling we have this snippet that is about to be unrolled:
<bb 3>:
# .MEM_13 = PHI <.MEM_8(6), .MEM_3(D)(2)>
# ivtmp_1 = PHI <ivtmp_10(6), 10(2)>
# .MEM_6 = VDEF <.MEM_13>
__transaction_relaxed
After loop unrolling the above remains unchanged, but the following is inserted
*before* bb 3. [Note, the following is after update_ssa(TODO_update_ssa), but
before cleanup_tree_cfg() for clarity.]
<bb 8>:
# .MEM_12 = PHI <.MEM_3(D)(2)>
# ivtmp_5 = PHI <10(2)>
# .MEM_6 = VDEF <.MEM_13> <-- *************************** -->
^^^^^^^^^^^^ <-- shouldn't this be <.MEM_12> ?????
<-- *************************** -->
__transaction_relaxed
...
...more unrolling
...
<bb 40>:
# .MEM_57 = PHI <.MEM_8(39)>
# ivtmp_58 = PHI <ivtmp_10(39)>
# .MEM_53 = VDEF <.MEM_13> <-- ************************** -->
<-- shouldn't this be <.MEM_57> ????
<-- ************************** -->
__transaction_relaxed
...
...
<bb 3>
..
Notice that both BB8 and BB40 invalidate MEM_13 in its virtual definition.
Shouldn't the VDEF<.MEM_13> be rewritten as I have indicated above? Especially
since now .MEM_13 is defined much further down in bb 3:
<bb 8>
# invalidates VDEF<.MEM_13>
...
<bb 40>
# invalidates VDEF<.MEM_13>
..
<bb 3>
# define .MEM_13 = ....
It seems gimple_duplicate_bb() renames all the phis and the operands, but does
not update the VDEF's for the transaction. Gimple_duplicate_bb() says it
doesn't maintain SSA form, so I assume this in on purpose (?). So how is this
supposed to be cleaned up? Or is this even the problem?
