On 05/15/2017 03:39 PM, Bernd Edlinger wrote:
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:
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)
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.
Well, my intention was actually to punt on those cases, but I hadn't
actually tested with -fsplit-stack. It looks like
ix86_expand_split_stack_prologue calls ix86_expand_call, and I hadn't
anticipated it getting called after the last call to
ix86_compute_frame_layout(), which your patch has probably eliminated.
In the case of -fsplit-stack, I'm testing the macro flag_split_stack
which (currently) just expands to check the global flag, so this could
instead be done in ix86_option_override_internal () instead, but I think
it highlights a somewhat deeper problem.
Rather or not m->call_ms2sysv is set determines which stack layout is
used when ix86_compute_frame_layout() runs. But if we can run
expand_call after the final time ix86_compute_frame_layout() then we
have a problem. It looks like ix86_expand_split_stack_prologue is the
only function that manually calls ix86_expand_call, but maybe it would
be better to modify the test to something like this:
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index a78819d6b3f..c36383f6962 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -29325,7 +29325,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx callarg1,
}
/* Set here, but it may get cleared later. */
- if (TARGET_CALL_MS2SYSV_XLOGUES)
+ if (TARGET_CALL_MS2SYSV_XLOGUES && !reload_completed)
cfun->machine->call_ms2sysv = true;
}
Or even use the same incompatibility tests from
ix86_compute_frame_layout (and possibly move them to a static).
But I didn't get an ICE when using a build from a few days ago. I've
updated to the current HEAD but I'm having problems with the test
failing even though there's no errors, as if expect has some finite
buffer for all of the warning spam and it kills the job. I'm running
another test to try and figure that out, but I have to run, so I'll get
back with you on the results.
Daniel
