Fwd: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Vladimir Yakovlev <vbyakovl23 at gmail dot com>]

---------- Forwarded message ----------
From: Vladimir Yakovlev <vbyakovl23@gmail.com>
Date: 2012/11/9
Subject: Re: [off-list] Re: [PATCH] Vzeroupper placement/47440
To: Uros Bizjak <ubizjak@gmail.com>
Копия: "H.J. Lu" <hjl.tools@gmail.com>, Igor Zamyatin <izamyatin@gmail.com>


I did changes that moves vzeroupper insertion after reload

2012-11-09  Vladimir Yakovlev  <vladimir.b.yakovlev@intel.com>

        * i386/i386-protos.h (ix86_avx256_optimize_mode_switching): New.
        * config/i386/i386.c (ix86_init_machine_status): Deleted
initialization for mode switching.
        * i386/i386.h (OPTIMIZE_MODE_SWITCHING1): New.
        * mode-switching.c (gate_mode_switching1): New.
        (rest_of_handle_mode_switching1): New.
        (pass_mode_switching1): New.
        * passes.c (init_optimization_passes): New pass pass_mode_switching1.
        * tree-pass.h (pass_mode_switching1): New.

But this caused assertion fails in  rtl_verify_flow_info_1 () at cfgrtl.c:2291

      fatal_insn ("flow control insn inside a basic block", x);

The asserts are called by two calls of mode-switching.c:
commit_edge_insertion and cleanup_cfg. After I commented (see below)
459.GemsFDTD benchspec passed. Your opinion of the patch and haw we
can do something with asserts.

Regards,
Vladimir

--- a/gcc/mode-switching.c
+++ b/gcc/mode-switching.c
@@ -747,7 +747,7 @@ optimize_mode_switching (void)
     commit_edge_insertions ();

 #if defined (MODE_ENTRY) && defined (MODE_EXIT)
-  cleanup_cfg (CLEANUP_NO_INSN_DEL);
+  /*cleanup_cfg (CLEANUP_NO_INSN_DEL);*/
 #else
   if (!need_commit && !emitted)
     return 0;
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -1828,7 +1828,7 @@ commit_edge_insertions (void)
   basic_block bb;

 #ifdef ENABLE_CHECKING
-  verify_flow_info ();
+  /*verify_flow_info ();*/
 #endif


2012/11/9 Uros Bizjak <ubizjak@gmail.com>:
> On Thu, Nov 8, 2012 at 6:52 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>
>> Uh, this is spill around call insn, produced by reload.
>>
>> Please compile this code:
>>
>> double test (double a)
>> {
>>   printf ("Hello\n");
>>   return a;
>> }
>>
>> You will get at mode switching:
>>
>>     1 NOTE_INSN_DELETED
>>     4 NOTE_INSN_BASIC_BLOCK
>>     2 r60:DF=xmm0:DF
>>       REG_DEAD: xmm0:DF
>>     3 NOTE_INSN_FUNCTION_BEG
>>     6 di:DI=`*.LC0'
>>     7 call <...>
>>       REG_DEAD: di:DI
>>       REG_UNUSED: ax:SI
>>    12 xmm0:DF=r60:DF
>>       REG_DEAD: r60:DF
>>    15 use xmm0:DF
>>
>> But reload will insert:
>>
>>     1 NOTE_INSN_DELETED
>>     4 NOTE_INSN_BASIC_BLOCK
>>     2 xmm0:DF=xmm0:DF
>>       REG_DEAD: xmm0:DF
>>    18 [sp:DI+0x8]=xmm0:DF
>>       REG_DEAD: xmm0:DF
>>     3 NOTE_INSN_FUNCTION_BEG
>>     6 di:DI=`*.LC0'
>>     7 call <...>
>>       REG_DEAD: di:DI
>>       REG_UNUSED: ax:SI
>>    19 xmm0:DF=[sp:DI+0x8]
>>       REG_DEAD: r62:DF
>>    12 xmm0:DF=xmm0:DF
>>       REG_DEAD: xmm0:DF
>>    15 use xmm0:DF
>>
>> I was not paying attention to this situation.
>
>
> A viable solution to this issue is through machine-reorg function (AKA
> x86_reorg) that would just move vzeroupper to the close proximity to a
> call insn. This would work on non-64bit-MS-ABI targets, where all SSE
> registers are dead at call insn place.
>
> Please note that 64bit-MS-ABI target declares registers xmm6+ as
> call-saved, so they can live over the call. I am not familiar with
> this target, but it looks to me that we have to remove vzeroupper, if
> one or more call-saved SSE registers are live at the call insn place.
>
> Uros.

Attachment: prvzu.patch
Description: Binary data