Hi,
This patch fixes computed gotos on m68k, and probably other targets too
(the fix is in the middle end). Several tests fail at present with "-O3
-fomit-frame-pointer".
One example of erroneous behaviour is as follows: the
function 'x' in comp-goto-1.c compiles to:
x:
lea (-104,%sp),%sp
lea (104,%sp),%a0
move.l %a0,92(%sp)
fmovem #63,44(%sp)
movem.l #31996,(%sp)
move.l %sp,96(%sp)
move.l 108(%sp),-(%sp)
lea (96,%sp),%a0
jsr y.1217
.L3:
.L12:
move.l 112(%sp),%d0 <--- wrong offset, should be 108.
movem.l (%sp),#31996
fmovem 44(%sp),#63
lea (104,%sp),%sp
rts
Where the source code looks like:
int
x(a)
{
__label__ xlab;
void y(a)
{
void *x = &&llab;
if (a==-1)
goto *x;
if (a==0)
goto xlab;
llab:
y (a-1);
}
y (a);
xlab:;
return a;
}
The offset is intended to load the argument value 'a' for the "return
a;" statement, but actually loads one word beyond that.
This has been broken because since mainline revision r160124, which
(correctly) causes 'y' to be flagged as noreturn -- y never returns via
the normal function return route. If ipa-pure-const.c is hacked to
check that a function does not perform non-local gotos before marking
it noreturn, the original (correct) offset is restored -- but that is
simply papering over the problem.
Prior to r160124, or with the aforementioned patch to inhibit the
"noreturn" attribute for the function y, the call to y from x (just
before "xlab:") has a goto to the following label emitted at expand
time. Before/after that patch is applied, the diff of 148r.expand
around the relevant part looks like:
# BLOCK 2 freq:10000
# PRED: ENTRY [100.0%] (fallthru,exec)
y (a_1(D)); [static-chain: &FRAME.0]
- # SUCC: 3 [100.0%] (ab,exec)
+ goto <bb 4> (xlab);
+ # SUCC: 3 [61.0%] (ab,exec) 4 [39.0%] (fallthru,exec)
- # BLOCK 3 freq:10000
- # PRED: 2 [100.0%] (ab,exec)
+ # BLOCK 3 freq:6100
+ # PRED: 2 [61.0%] (ab,exec)
<L0>: [non-local]
# SUCC: 4 [100.0%] (fallthru,exec)
# BLOCK 4 freq:10000
- # PRED: 3 [100.0%] (fallthru,exec)
+ # PRED: 2 [39.0%] (fallthru,exec) 3 [100.0%] (fallthru,exec)
xlab:
Now, this is important because when the "goto <bb 4>" is emitted, a
stack adjustment is also emitted to pop y's arguments (via
dojump.c:do_pending_stack_adjust). Without that (i.e. the present state
with no patches), when later passes try to figure out elimination
offsets (from arg pointer to frame pointer, frame pointer to stack
pointer and so on) at the labels L0/xlab, they get the wrong answer.
(This happens in reload, called from ira.)
So, on to the attached fix. As mentioned previously, GCC tracks
elimination offsets throughout each function, which can vary as e.g.
other functions are called. There is a concept of an "initial" offset:
I believe that when nonlocal gotos are executed, and hence at all the
labels a nonlocal goto may reach, the elimination offsets must have
their "initial" values.
We can find out if a label is the target of a nonlocal goto (and hence
force the use of initial offsets) in a somewhat roundabout way, by
looking up its containing basic block, seeing if that BB is a nonlocal
goto target, then seeing if the label is the first instruction in the
block. This seems slightly clumsy, but I didn't find another way of
doing it.
Interestingly perhaps, the comment near the fix in
reload1.c:set_label_offsets hints at a possible issue which may be
related:
case CODE_LABEL:
/* If we know nothing about this label, set the desired offsets.
Note that this sets the offset at a label to be the offset before a
label if we don't know anything about the label. This is not
correct for the label after a BARRIER, but is the best guess we can
make. If we guessed wrong, we will suppress an elimination that
might have been possible had we been able to guess correctly. */
But obviously, in our case, rather than just failing to make an
elimination which may have been possible, we get wrong code instead
(there is a barrier before our non-local-goto-receiving label).
Anyway. The attached patch appears to work fine, and we get the
following changes in test results (cross-building and testing to
ColdFire Linux):
FAIL -> PASS: gcc.c-torture/execute/920501-7.c execution, -O3
-fomit-frame-pointer
FAIL -> PASS: gcc.c-torture/execute/comp-goto-2.c execution, -O3
-fomit-frame-pointer
FAIL -> PASS: gcc.dg/torture/stackalign/comp-goto-1.c -O3 -fomit-frame-pointer
execution test
FAIL -> PASS: gcc.dg/torture/stackalign/comp-goto-1.c -O3 -fomit-frame-pointer
execution test [#2]
FAIL -> PASS: gcc.dg/torture/stackalign/non-local-goto-4.c -O3
-fomit-frame-pointer execution test
FAIL -> PASS: gcc.dg/torture/stackalign/non-local-goto-4.c -O3
-fomit-frame-pointer execution test [#2]
Any comments, or OK to apply?
Thanks,
Julian
ChangeLog
gcc/
* reload1.c (set_label_offsets): Force initial_p for labels
receiving non-local gotos.
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c (revision 179967)
+++ gcc/reload1.c (working copy)
@@ -2371,6 +2371,19 @@ set_label_offsets (rtx x, rtx insn, int
if (! offsets_known_at[CODE_LABEL_NUMBER (x) - first_label_num])
{
+ if (x == insn)
+ {
+ basic_block bb;
+
+ bb = BLOCK_FOR_INSN (insn);
+
+ /* If the label is the target of a non-local GOTO, we must use
+ the initial elimination offsets. */
+ if (bb && BB_HEAD (bb) == insn
+ && (bb->flags & BB_NON_LOCAL_GOTO_TARGET))
+ initial_p = true;
+ }
+
for (i = 0; i < NUM_ELIMINABLE_REGS; i++)
offsets_at[CODE_LABEL_NUMBER (x) - first_label_num][i]
= (initial_p ? reg_eliminate[i].initial_offset
