diff mbox

[i386] Recompute the frame layout less often

Message ID AM4PR0701MB2162B662041232E3531A8E07E4E10@AM4PR0701MB2162.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger May 15, 2017, 8:39 p.m. UTC 
On 05/15/17 03:39, Daniel Santos wrote:
> On 05/14/2017 11:31 AM, Bernd Edlinger wrote:

>> Hi Daniel,

>>

>> there is one thing I don't understand in your patch:

>> That is, it introduces a static value:

>>

>> /* Registers who's save & restore will be managed by stubs called from

>>      pro/epilogue.  */

>> static HARD_REG_SET GTY(()) stub_managed_regs;

>>

>> This seems to be set as a side effect of ix86_compute_frame_layout,

>> and depends on the register usage of the current function.

>> But values that depend on the current function need usually be

>> attached to cfun->machine, because the passes can run in parallel

>> unless I am completely mistaken, and the stub_managed_regs may

>> therefore be computed from a different function.

>>

>>

>> Bernd.

>

> I should add that if you want to run faster tests just on the ms to sysv

> abi code, you can use make RUNTESTFLAGS="ms-sysv.exp" check and then if

> that succeeds run the full testsuite.

>

> Daniel


Unfortunately I encounter a serious problem when my patch is used
ontop of your patch, Yes, the test suite ran without error, but then
I tried to trigger the warning and that tripped an ICE.
The reason is that cfun->machine->call_ms2sysv can be set to true
*after* reload_completed, which can be seen using the following
patch:



That assertion is triggered in this test case:

cat test.c
int test()
{
   __builtin_printf("test\n");
   return 0;
}

gcc -mabi=ms -mcall-ms2sysv-xlogues -fsplit-stack -c test.c
test.c: In function 'test':
test.c:5:1: internal compiler error: in ix86_expand_call, at 
config/i386/i386.c:29324
  }
  ^
0x13390a4 ix86_expand_call(rtx_def*, rtx_def*, rtx_def*, rtx_def*, 
rtx_def*, bool)
	../../gcc-trunk/gcc/config/i386/i386.c:29324
0x1317494 ix86_expand_split_stack_prologue()
	../../gcc-trunk/gcc/config/i386/i386.c:15920
0x162ba21 gen_split_stack_prologue()
	../../gcc-trunk/gcc/config/i386/i386.md:12556
0x12f3f30 target_gen_split_stack_prologue
	../../gcc-trunk/gcc/config/i386/i386.md:12325
0xb237b3 make_split_prologue_seq
	../../gcc-trunk/gcc/function.c:5822
0xb23a08 thread_prologue_and_epilogue_insns()
	../../gcc-trunk/gcc/function.c:5958
0xb24840 rest_of_handle_thread_prologue_and_epilogue
	../../gcc-trunk/gcc/function.c:6428
0xb248c0 execute
	../../gcc-trunk/gcc/function.c:6470
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.


so, in ix86_expand_split_stack_prologue
we first call:
   ix86_finalize_stack_realign_flags ();
   ix86_compute_frame_layout (&frame);

and later:
   call_insn = ix86_expand_call (NULL_RTX, gen_rtx_MEM (QImode, fn),
                                 GEN_INT (UNITS_PER_WORD), constm1_rtx,
                                 pop, false);

which changes a flag with a huge impact on the frame layout, but there
is no absolutely no way how the frame layout can change once it is
finalized.


Any Thoughts?


Bernd.
diff mbox

Patch

Index: i386.c
===================================================================
--- i386.c	(revision 248031)
+++ i386.c	(working copy)
@@ -29320,7 +29320,10 @@ 

        /* Set here, but it may get cleared later.  */
        if (TARGET_CALL_MS2SYSV_XLOGUES)
+      {
+	gcc_assert(!reload_completed);
  	cfun->machine->call_ms2sysv = true;
+      }
      }

    if (vec_len > 1)