This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix PR middle-end/24003
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Fri, 11 Nov 2005 11:10:26 +0100
- Subject: [PATCH] Fix PR middle-end/24003
Hi,
This is the miscompilation of the Ada runtime (s-arit64.adb) on i686 at -O and
above, C testcase attached.
The statement in the C testcase
concat (lo (p1), p2);
is miscompiled at -O -mtune=i686 -fno-inline.
Excerpt from the assembly file:
movl %eax, (%esp)
movl %edx, 4(%esp)
call lo
movl %ebx, (%esp)
movl %esi, 4(%esp)
call concat
The return value of lo is discarded.
Excerpt from t97.final_cleanup:
<bb 0>:
return concat (lo (p1), p2);
As pointed out by Jan, TER is responsible for the rematerialization of the
nested calls. In other words, the bug vanishes if -fno-tree-ter.
Excerpt from 00.expand:
(call_insn/u 13 12 14 1 (set (reg:SI 0 ax)
(call (mem:QI (symbol_ref:SI ("lo") [flags 0x3] <function_decl
0x5574ff00 lo>) [0 S1 A8])
(const_int 8 [0x8]))) -1 (nil)
(expr_list:REG_EH_REGION (const_int 0 [0x0])
(nil))
(expr_list:REG_DEP_TRUE (use (mem/i:DI (reg/f:SI 56 virtual-outgoing-args)
[0 S8 A32]))
(nil)))
(insn 14 13 15 1 (set (mem/i:SI (reg/f:SI 56 virtual-outgoing-args) [0 S4
A32])
(reg:SI 0 ax)) -1 (nil)
(nil))
(insn 15 14 16 1 (set (mem:DI (reg/f:SI 56 virtual-outgoing-args) [0 S8 A8])
(reg:DI 62)) -1 (nil)
(nil))
(call_insn/u 16 15 17 1 (set (reg:DI 0 ax)
(call (mem:QI (symbol_ref:SI ("concat") [flags 0x3] <function_decl
0x5574ff80 concat>) [0 S1 A8])
(const_int 8 [0x8]))) -1 (nil)
(expr_list:REG_EH_REGION (const_int 0 [0x0])
(nil))
(expr_list:REG_DEP_TRUE (use (mem/i:SI (reg/f:SI 56 virtual-outgoing-args)
[0 S4 A32]))
(expr_list:REG_DEP_TRUE (use (mem/i:SI (plus:SI (reg/f:SI 56
virtual-outgoing-args)
(const_int 4 [0x4])) [0 S4 A32]))
(nil))))
The return value is saved in the target:
(insn 14 13 15 1 (set (mem/i:SI (reg/f:SI 56 virtual-outgoing-args) [0 S4
A32])
(reg:SI 0 ax)) -1 (nil)
(nil))
before the saved arguments are restored:
(insn 15 14 16 1 (set (mem:DI (reg/f:SI 56 virtual-outgoing-args) [0 S8 A8])
(reg:DI 62)) -1 (nil)
(nil))
The naive fix, i.e. restoring the saved arguments before saving the return
value, seems to work fine on i686, though I'm not really sure that's true
everywhere.
Boostrapped/regtested on i686-suse-linux. OK for mainline?
2005-11-11 Eric Botcazou <ebotcazou@adacore.com>
PR middle-end/24003
* calls.c (expand_call): Save the return value in the target last.
2005-11-11 Eric Botcazou <ebotcazou@adacore.com>
* gcc.dg/nested-calls-1.c: New test.
--
Eric Botcazou
Index: calls.c
===================================================================
--- calls.c (revision 106739)
+++ calls.c (working copy)
@@ -2807,6 +2807,57 @@ expand_call (tree exp, rtx target, int i
}
}
+ /* If size of args is variable or this was a constructor call for a stack
+ argument, restore saved stack-pointer value. */
+
+ if (old_stack_level && ! (flags & ECF_SP_DEPRESSED))
+ {
+ emit_stack_restore (SAVE_BLOCK, old_stack_level, NULL_RTX);
+ stack_pointer_delta = old_stack_pointer_delta;
+ pending_stack_adjust = old_pending_adj;
+ old_stack_allocated = stack_pointer_delta - pending_stack_adjust;
+ stack_arg_under_construction = old_stack_arg_under_construction;
+ highest_outgoing_arg_in_use = initial_highest_arg_in_use;
+ stack_usage_map = initial_stack_usage_map;
+ sibcall_failure = 1;
+ }
+ else if (ACCUMULATE_OUTGOING_ARGS && pass)
+ {
+#ifdef REG_PARM_STACK_SPACE
+ if (save_area)
+ restore_fixed_argument_area (save_area, argblock,
+ high_to_save, low_to_save);
+#endif
+
+ /* If we saved any argument areas, restore them. */
+ for (i = 0; i < num_actuals; i++)
+ if (args[i].save_area)
+ {
+ enum machine_mode save_mode = GET_MODE (args[i].save_area);
+ rtx stack_area
+ = gen_rtx_MEM (save_mode,
+ memory_address (save_mode,
+ XEXP (args[i].stack_slot, 0)));
+
+ if (save_mode != BLKmode)
+ emit_move_insn (stack_area, args[i].save_area);
+ else
+ emit_block_move (stack_area, args[i].save_area,
+ GEN_INT (args[i].locate.size.constant),
+ BLOCK_OP_CALL_PARM);
+ }
+
+ highest_outgoing_arg_in_use = initial_highest_arg_in_use;
+ stack_usage_map = initial_stack_usage_map;
+ }
+
+ /* If this was alloca, record the new stack level for nonlocal gotos.
+ Check for the handler slots since we might not have a save area
+ for non-local gotos. */
+
+ if ((flags & ECF_MAY_BE_ALLOCA) && cfun->nonlocal_goto_save_area != 0)
+ update_nonlocal_goto_save_area ();
+
/* If value type not void, return an rtx for the value. */
if (TYPE_MODE (TREE_TYPE (exp)) == VOIDmode
@@ -2915,57 +2966,6 @@ expand_call (tree exp, rtx target, int i
}
}
- /* If size of args is variable or this was a constructor call for a stack
- argument, restore saved stack-pointer value. */
-
- if (old_stack_level && ! (flags & ECF_SP_DEPRESSED))
- {
- emit_stack_restore (SAVE_BLOCK, old_stack_level, NULL_RTX);
- stack_pointer_delta = old_stack_pointer_delta;
- pending_stack_adjust = old_pending_adj;
- old_stack_allocated = stack_pointer_delta - pending_stack_adjust;
- stack_arg_under_construction = old_stack_arg_under_construction;
- highest_outgoing_arg_in_use = initial_highest_arg_in_use;
- stack_usage_map = initial_stack_usage_map;
- sibcall_failure = 1;
- }
- else if (ACCUMULATE_OUTGOING_ARGS && pass)
- {
-#ifdef REG_PARM_STACK_SPACE
- if (save_area)
- restore_fixed_argument_area (save_area, argblock,
- high_to_save, low_to_save);
-#endif
-
- /* If we saved any argument areas, restore them. */
- for (i = 0; i < num_actuals; i++)
- if (args[i].save_area)
- {
- enum machine_mode save_mode = GET_MODE (args[i].save_area);
- rtx stack_area
- = gen_rtx_MEM (save_mode,
- memory_address (save_mode,
- XEXP (args[i].stack_slot, 0)));
-
- if (save_mode != BLKmode)
- emit_move_insn (stack_area, args[i].save_area);
- else
- emit_block_move (stack_area, args[i].save_area,
- GEN_INT (args[i].locate.size.constant),
- BLOCK_OP_CALL_PARM);
- }
-
- highest_outgoing_arg_in_use = initial_highest_arg_in_use;
- stack_usage_map = initial_stack_usage_map;
- }
-
- /* If this was alloca, record the new stack level for nonlocal gotos.
- Check for the handler slots since we might not have a save area
- for non-local gotos. */
-
- if ((flags & ECF_MAY_BE_ALLOCA) && cfun->nonlocal_goto_save_area != 0)
- update_nonlocal_goto_save_area ();
-
/* Free up storage we no longer need. */
for (i = 0; i < num_actuals; ++i)
if (args[i].aligned_regs)
/* PR middle-end/24003 */
/* Contributed by Eric Botcazou <ebotcazou@adacore.com> */
/* { dg-do run } */
/* { dg-options "-std=c99 -O -fno-inline" } */
/* { dg-options "-std=c99 -O -fno-inline -mtune=i686" { target { i?86-*-* && ilp32 } } } */
#include <limits.h>
typedef unsigned long uns32_t;
typedef unsigned long long uns64_t;
extern void abort(void);
uns32_t lo (uns64_t p)
{
return (uns32_t)p;
}
uns64_t concat (uns32_t p1, uns32_t p2)
{
#if LLONG_MAX > 2147483647L
return ((uns64_t)p1 << 32) | p2;
#else
return 0;
#endif
}
uns64_t lshift32 (uns64_t p1, uns32_t p2)
{
return concat (lo (p1), p2);
}
int main(void)
{
#if LLONG_MAX > 2147483647L
if (lshift32 (0xFFFFFFFF12345678ULL, 0x90ABCDEFUL) != 0x1234567890ABCDEFULL)
abort ();
#endif
return 0;
}