[PATCH] Fix ICE in match_asm_constraints_1 (PR inline-asm/84941)

[Michael Matz]

Hi,

On Mon, 19 Mar 2018, Jakub Jelinek wrote:

> > Blaeh.  Note that "1p" is actually invalid:
> > 
> > --------------
> > `0', `1', `2', ... `9'
> >      An operand that matches the specified operand number is allowed.
> >      If a digit is used together with letters within the same
> >      alternative, the digit should come last.
> > --------------
> > 
> > Changing the order to "p1" would disable the transformation as well, 
> > because match_asm_constraints_1() uses strtoul on the constraint start.
> 
> Ah, ok, but
>   asm volatile ("" : "=m,m" (b), "=r,r" (b) : "1,p" (b));
> ICEs the same way, and that should be valid even according to the above
> description.

Yes that's valid and shouldn't ICE.

> > ... this makes sense.  But I think you're too generous in supporting 
> > strange inputs:
> > 
> > >        if (! REG_P (output)
> > >  	  || rtx_equal_p (output, input)
> > >  	  || (GET_MODE (input) != VOIDmode
> > > -	      && GET_MODE (input) != GET_MODE (output)))
> > > +	      && GET_MODE (input) != GET_MODE (output))
> > > +	  || !(REG_P (input) || SUBREG_P (input)
> > > +	       || MEM_P (input) || CONSTANT_P (input)))
> > 
> > I'd only allow REG_P (input) as well, not any of the other forms.
> 
> I'll try to gather some statistics on what kind of inputs appear there
> during bootstrap/regtest and will try to write a few testcases to see
> if just || ! REG_P (output) is sufficient or not.

I think you don't need to try too hard.  The function is designed to 
handle the situation where the matching constraint is a result from an 
input-output constraint, not for improving arbitrary matching constraints.
As such the expected situation is that input and output are lvalues, and 
hence (when their type is anything sensible) gimple registers, and 
therefore pseudos at RTL time.

You could basically reject any constraint that isn't just a single integer 
(i.e. anything with not only digits after the optional '%') and still 
handle the sitatuations for which this function was invented.  I.e. like 
this:

Index: function.c
===================================================================
--- function.c  (revision 254008)
+++ function.c  (working copy)
@@ -6605,7 +6605,7 @@ match_asm_constraints_1 (rtx_insn *insn,
        constraint++;
 
       match = strtoul (constraint, &end, 10);
-      if (end == constraint)
+      if (end == constraint || *end)
        continue;
 
       gcc_assert (match < noutputs);



Ciao,
Michael.