Re: [PATCH, dataflow]: Fix PR55845, 454.calculix miscompares on x86 AVX due to movement of vzeroupper

[Jakub Jelinek <jakub@redhat.com>]

On Sun, Jan 06, 2013 at 04:48:03PM +0100, Uros Bizjak wrote:
> --- df-problems.c	(revision 194945)
> +++ df-problems.c	(working copy)
> @@ -3916,6 +3916,10 @@ can_move_insns_across (rtx from, rtx to, rtx acros
>  	break;
>        if (NONDEBUG_INSN_P (insn))
>  	{
> +	  /* Do not move unspec_volatile insns.  */
> +	  if (GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE)
> +	    break;
> +

Shouldn't UNSPEC_VOLATILE be handled similarly in the across_from ..
across_to loop?  Both UNSPEC_VOLATILE and volatile asm are handled there
just with
	trapping_insns_in_across |= may_trap_p (PATTERN (insn));
but your new change doesn't prevent moving just trapping insns across
UNSPEC_VOLATILE, but any insns whatsoever.  So supposedly for UNSPEC_VOLATILE
the first loop should just return false; (or fail = 1; ?).
For asm volatile I guess the code is fine as is, it must always describe
what exactly it modifies, so supposedly non-trapping insns can be moved
across asm volatile.

>  	  if (may_trap_or_fault_p (PATTERN (insn))
>  	      && (trapping_insns_in_across || other_branch_live != NULL))
>  	    break;

You could do the check only for may_trap_or_fault_p, all UNSPEC_VOLATILE
may trap.

BTW, can't UNSPEC_VOLATILE be embedded deeply in the pattern?
So volatile_insn_p (insn) && asm_noperands (PATTERN (insn)) == -1?
But perhaps you want to treat that way only UNSPEC_VOLATILE directly in the
pattern and all other UNSPEC_VOLATILE insns must describe in detail what
exactly they are changing?  This really needs to be better documented.

	Jakub