Hi,
this is a regression present on all active branches: if you compile the
attached Ada testcase with -O2 -gnatp -fno-omit-frame-pointer for 32-bit
Windows, you'll see that the compiler swaps a load based on the stack pointer
with a store based on the frame pointer, thus clobbering a saved argument:
pushl %ebp
movl %esp, %ebp
pushl %esi
pushl %ebx
pushl %eax <- %eax save
movl $4108, %eax
call ___chkstk_ms
leal 8(%ebp), %esi
subl %eax, %esp
movl 8(%ebp), %ebx
movl %edx, -20(%ebp)
movl %esi, -12(%ebp) <- fp-based store
movl (%esp,%eax), %eax <- sp-based load
... wrong value in eax...
The load and the store are swapped because there are not based on the same
register and the offset between them is seen as variable. The proposed fix is
to add a memory blockage, like in other frame-related constructs.
Tested on x86/Windows and x86-64/Windows, OK for all active branches?
2019-02-06 Eric Botcazou <ebotca...@adacore.com>
* config/i386/i386.c (ix86_expand_prologue): Generate a memory blockage
after restoring registers saved to allocate the frame on Windows.
2019-02-06 Eric Botcazou <ebotca...@adacore.com>
* gnat.dg/opt76.adb: New test.
--
Eric Botcazou
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 268508)
+++ config/i386/i386.c (working copy)
@@ -13579,8 +13579,9 @@ ix86_expand_prologue (void)
}
m->fs.sp_offset += allocate;
- /* Use stack_pointer_rtx for relative addressing so that code
- works for realigned stack, too. */
+ /* Use stack_pointer_rtx for relative addressing so that code works for
+ realigned stack. But this means that we need a blockage to prevent
+ stores based on the frame pointer from being scheduled before. */
if (r10_live && eax_live)
{
t = gen_rtx_PLUS (Pmode, stack_pointer_rtx, eax);
@@ -13589,6 +13590,7 @@ ix86_expand_prologue (void)
t = plus_constant (Pmode, t, UNITS_PER_WORD);
emit_move_insn (gen_rtx_REG (word_mode, AX_REG),
gen_frame_mem (word_mode, t));
+ emit_insn (gen_memory_blockage ());
}
else if (eax_live || r10_live)
{
@@ -13596,6 +13598,7 @@ ix86_expand_prologue (void)
emit_move_insn (gen_rtx_REG (word_mode,
(eax_live ? AX_REG : R10_REG)),
gen_frame_mem (word_mode, t));
+ emit_insn (gen_memory_blockage ());
}
}
gcc_assert (m->fs.sp_offset == frame.stack_pointer_offset);
-- { dg-do run }
-- { dg-options "-O2 -gnatp -fno-omit-frame-pointer" }
procedure Opt76 is
type Integer_Access is access Integer;
type Registry_Array is array (Natural range <>) of Integer_Access;
procedure Nested (Input, Parser : Integer; A, B : Boolean) is
Index : Registry_Array (1 .. 1024);
Not_B : constant Boolean := not B;
procedure Inner (Input : Integer) is
begin
if Input /= 1 then
raise Program_Error;
end if;
if Parser = 128 and then A and then Not_B then
Inner (Input);
Index (Index'First) := null;
end if;
end;
begin
Inner (Input);
end;
Input : Integer := 1 with Volatile;
Parser : Integer := 2 with Volatile;
begin
Nested (Input, Parser, False, True);
Nested (Input, Parser, True, False);
end;