Re: [PATCH] Fix -O0 -ffloat-store -mavx ICE (PR target/79807)

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Mar 03, 2017 at 12:18:09PM +0100, Uros Bizjak wrote:
> > as the constraint require that both operands aren't memory, shall I create a
> > patch for that?  This is the first category below.
> 
> Yes. Although expander takes care not to generate two memory
> references, combine can propagate memory to the other operand,
> creating semi-invalid RTX that is later resolved by RA.

Ok, will work on a patch.

> > The second category is where matching operand is ok, so my patch can
> > pessimize stuff.  I wonder if we couldn't handle this like:
> >           /* If we aren't optimizing, only allow one memory operand to be
> >              generated.  */
> >           if (memory_operand (op, mode))
> >             {
> >               const char *const constraint
> >                 = insn_data[icode].operand[i + adjust + 1].constraint;
> >               if (optimize
> >                   || num_memory != 1
> >                   || !rtx_equal_p (real_target, op))
> >                 num_memory++;
> >               /* sse2_movsd allows matching operand.  */
> >               else if (icode == CODE_FOR_sse2_movsd)
> >                 ;
> >               /* Various masked insns allow matching operand.  */
> >               else if (insn_data[icode].operand[i + adjust + 1].predicate
> >                        == vector_move_operand
> >                        && (strcmp (constraint, "0C") == 0
> >                            || strcmp (constraint, "0C,0") == 0))
> >                 ;
> >               else
> >                 num_memory++;
> >             }
> > (though perhaps sse2_movsd is still too simplistic, because it has just
> > =m v 0 and =o 0 v alternatives, so if i + adjust + 1 is 2, then it is
> > fine as is, if it is 1, then only if the memory is offsettable; though
> > perhaps LRA can just turn non-offsettable memory into offsettable one
> > through secondary reload).
> 
> There is similarity with arithmetic operations. Please note that arith
> ops fixup their arguments using ix86_expand_binary_operator, and their
> insn patterns have to satisfy ix86_binary_operator_ok insn predicate.
> However, even using this infrastructure, I have noticed that the
> compiler is quite sloppy with memory operands, and we mostly get
> matched memory after combine pass (if we are lucky).
> 
> Regarding the implementation, I'd rather see another flag for
> i386-builtin.def entries, where we can mark if matching memory operand
> is allowed. The expander can then look at the flag, and do some
> special magic.

I think maintaining such a flag would be a nightmare, I think more insns
will need it in the future and it will be hard to remember.  Maybe it is better
not to do anything about those (i.e. apply my patch as is), after all -O0
is not optimized code.

	Jakub