[i386] : Fix PR 59021, new vzeroupper instructions generated with -mavx

On Wed, Nov 6, 2013 at 11:06 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Attached patch fixes PR 59021, where new vzeroupper instructions are
>> generated for -mavx after Vlad's useless insn removal patch.
>>
>> The problem was, that we depent on (useless) moves to AVX256 function
>> argument registers in front of the function call to switch the mode to
>> DIRTY mode. This is not true anymore, so call_insn has to switch to
>> DIRTY mode by itself.
>
> Thanks for fixing this!

OTOH, looking a bit deeper, it looks that there is a problem in
mode-switching infrastructure. If we have a BB without any mode
requirements, but an insn that switched the mode via MODE_AFTER, we
should not mark the block as transparent. Indeed, we even have a N.B.
comment in mode-switching.c:

      /* Check for blocks without ANY mode requirements.
         N.B. because of MODE_AFTER, last_mode might still be different
         from no_mode.  */

But, we do nothing w.r.t to transparency

Attached incremental patch also fixes additional vzeroupper problem.
Calls with AVX argument register in fact do not have any mode
requirements, so we don't need to fake MODE_DIRTY requirement.

I have added Joern to the discussion.

Uros.

On 7 November 2013 20:01, Uros Bizjak <ubizjak@gmail.com> wrote:

> OTOH, looking a bit deeper, it looks that there is a problem in
> mode-switching infrastructure. If we have a BB without any mode
> requirements, but an insn that switched the mode via MODE_AFTER, we
> should not mark the block as transparent. Indeed, we even have a N.B.
> comment in mode-switching.c:
>
>       /* Check for blocks without ANY mode requirements.
>          N.B. because of MODE_AFTER, last_mode might still be different
>          from no_mode.  */
>
> But, we do nothing w.r.t to transparency

The optimize_mode_switching patch makes sense to me.

> Attached incremental patch also fixes additional vzeroupper problem.
> Calls with AVX argument register in fact do not have any mode
> requirements, so we don't need to fake MODE_DIRTY requirement.

Not sure about this - because I'm note sure what the meaning of values in
avx_u128_state are.  This lack of clarity is not caused by your  patch,
but is that something that could be reasonably be explained
in a comment, or would that have to be too long to be helpful?

On Fri, Nov 8, 2013 at 1:07 AM, Joern Rennecke
<joern.rennecke@embecosm.com> wrote:
> On 7 November 2013 20:01, Uros Bizjak <ubizjak@gmail.com> wrote:
>
>> OTOH, looking a bit deeper, it looks that there is a problem in
>> mode-switching infrastructure. If we have a BB without any mode
>> requirements, but an insn that switched the mode via MODE_AFTER, we
>> should not mark the block as transparent. Indeed, we even have a N.B.
>> comment in mode-switching.c:
>>
>>       /* Check for blocks without ANY mode requirements.
>>          N.B. because of MODE_AFTER, last_mode might still be different
>>          from no_mode.  */
>>
>> But, we do nothing w.r.t to transparency
>
> The optimize_mode_switching patch makes sense to me.

Thanks, I will propose a patch for this.

>> Attached incremental patch also fixes additional vzeroupper problem.
>> Calls with AVX argument register in fact do not have any mode
>> requirements, so we don't need to fake MODE_DIRTY requirement.
>
> Not sure about this - because I'm note sure what the meaning of values in
> avx_u128_state are.  This lack of clarity is not caused by your  patch,
> but is that something that could be reasonably be explained
> in a comment, or would that have to be too long to be helpful?

No problem here, these are state transitions for vzeroupper
mode-switching state. State should be changed to DIRTY by functions
that use AVX256 argument registers.

Best regards,
Uros.