This is the mail archive of the
mailing list for the GCC project.
On 07/19/10 12:39, Maxim Kuvyrkov wrote:
But you don't know the target block for the insertion when you're
pruning the data sets. Thus you don't know if the target block has any
of the odd situations which require insertion earlier in the block.
hoist_expr_reaches_here_p is supposed to determine if an expression, if
inserted at the "end" of a specific block reaches a later point in the
cfg unchanged. Since we know the target block of the insertion we can
check for the various odd conditions that require insertion before the
actual end of the target block.
Just so I'm certain I understand the issue. Is the problem that the
CALL_INSN is clobbering the value in the MEM, which normally wouldn't be
a problem except that we can (in certain cases) place the MEM before the
One could argue this is a failing of
hoist_expr_reaches_here_p which fails to take the corner cases of
inserting at the end of a block into account.
While that's technically correct, I see hoist_expr_reaches_here_p as
designed to perform it's job using antloc, transp, and other data
sets, i.e., without examining the instruction stream. It seems
simpler to handle corner cases like this one by cleaning up the data
sets earlier in the pass.
The fact that we haven't stumbled across this bug doesn't mean the
problem does not exist. I think we've largely been lucky because we
don't run code hoisting unless optimizing for size and because the
combiner is the pass most likely to create these insns and the combiner
runs after gcse.
Looking at insert_insn_end_basic_block we have another class of
problems. Say the target block ends with a set & jump insn. The
destination of the set might clobber a value needed by an insn hoisted
to the end of the block. What a mess we've stumbled into.
In theory, "yes" it may happen that a set and/or jump insns modify
memory or have other interesting side effects. In practice, we don't
seem to have this problem with any of the ports.
I'm not saying you have to address this problem right now (though a
comment discussing the issue would be greatly appreciated).
I mis-understood what the code was checking for -- it's dealing with
MEMs which are known to be constant and thus can't be clobbered by the
call, which clearly isn't a problem. My bad.
+ if (!pre_p && MEM_P (e->expr))
+ /* Note memory references that can be clobbered by a call.
+ We do not split abnormal edges in HOIST, so would
+ a memory reference get hoisted along an abnormal edge,
+ it would be placed /before/ the call. Therefore, only
+ constant memory references can be hoisted along abnormal
+ edges. */
+ if (GET_CODE (XEXP (e->expr, 0)) == SYMBOL_REF
+ && CONSTANT_POOL_ADDRESS_P (XEXP (e->expr, 0)))
This is the only part I'm struggling with. It looks like you're trying
to avoid setting prune_exprs if the MEM's address has certain properties
(such as a reference to static memory). However, if my analysis of the
problem is correct, that's not sufficient to solve the problem.
This code comes straight from compute_transpout.
I think at this point we should go forward with your patch and if you
could add a pair of comments to expr_reaches_here_p and the pruning code
which touch on the problems with set-and-jump insns as a separate patch,
that would be greatly appreciated.
Thanks for your patience,