This is the mail archive of the
mailing list for the GCC project.
On 7/19/10 10:08 PM, Jeff Law wrote:
On 07/17/10 10:41, Maxim Kuvyrkov wrote:
On 7/16/10 10:37 PM, Jeff Law wrote:
Placement before the call is intentional.
On 07/15/10 13:22, Maxim Kuvyrkov wrote:
Yes as long as it passes the usual bootstrap & regression test.
On 7/15/10 8:06 PM, Jeff Law wrote:
? See the e->flags & EDGE_ABNORMAL test prior to removing elements of
trapping_expr from antloc or transp.
I missed that point that we should avoid emitting anything at the ends
of basic blocks with abnormal edges.
Is the attached patch OK?
Well, after the regtest I now know the exact purpose of transpout. It
is to avoid moving memory references across calls that can clobber
memory. Without the checks done in compute_transpout a memory
reference can be moved along an abnormal edge and, when that happens,
it gets placed /before/ the call.
The CALL_INSN (if it has
abnormal edges) has to be considered to change control flow. If the
hoisted insn is placed after the CALL_INSN, the hoisted insn may not be
executed if the CALL_INSN throws (for example).
insert_insn_end_basic_block contains the logic to determine where in a
block to place the new insn.
I *think* PRE avoids this by only inserting in a block where it knows
the inserted insn is safe to place anywhere within the block. In the
case of a MEM, we must have already determined that the block (and thus
any CALL_INSN within the block) doesn't clobber the MEM. Hoisting is a
little different in that it only verifies that it should be safe to
place the MEM at the end of 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.
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.
+ 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.
Why do you think these checks aren't sufficient? A memory reference to
a constant pool is, well, a constant.
It may be that the second check
if (MEM_READONLY_P (e->expr)
&& !MEM_VOLATILE_P (e->expr)
&& MEM_NOTRAP_P (e->expr))
/* Constant memory reference, e.g., a PIC address. */
implies CONSTANT_POOL_ADDRESS_P, but I prefer to err on the side of
caution have both checks in place.
(650) 331-3385 x724