On Mon, Mar 19, 2018 at 08:38:00PM +0000, Michael Matz wrote:
> > 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.
I will change the testcase in the patch to the above to make it valid.
> > > > 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.
It is very common that input is one of the above cases, during x86_64-linux
and i686-linux bootstraps+regtests I got:
13201x CONST_INT, 1959x MEM, 114x SUBREG, 110x SYMBOL_REF,
2x PLUS (the new testcase only)
and most of those were actually from input-output constraints, like:
var = 1;
asm ("" : "+g" (var));
var2 = &static_var3;
asm ("" : "+g" (var2));
etc. I believe the mini-pass does a useful transformation for these that
ought to make it easier for reload to actually handle the matching constraints.
Consider:
long a, b, c, d;
long
f1 (void)
{
long r = 1;
long s = 2;
long t = 3;
long u = 4;
asm volatile ("" : "+g" (r), "+g" (s), "+g" (t), "+g" (u)
: : "ax", "bx", "cx", "dx", "bp", "si", "di",
"r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
"xmm0", "xmm1", "xmm2", "xmm3", "xmm4", "xmm5", "xmm6",
"xmm7",
"xmm8", "xmm9", "xmm10", "xmm11", "xmm12", "xmm13",
"xmm14", "xmm15");
return r + s + t + u;
}
This ought to be reloadable, by spilling r, s, t, u into stack slots and
using NN(%rsp) for those, and the patch with the
|| !(REG_P (input) || SUBREG_P (input)
|| MEM_P (input) || CONSTANT_P (input)))
rather than just
|| ! REG_P (input))
attempts to help that by forcing the constants into the output pseudo.
Similarly for &static_var (SYMBOL_REF), loads from some memory (MEM),
or e.g. generic vector casts (SUBREG). Seems LRA gives up on it anyway,
which to me looks like a LRA bug (it works with "+rm" instead of "+g").
Tried also:
long a, b, c, d;
long
f1 (void)
{
long r = 1;
long s = 2;
long t = 3;
long u = 4;
asm volatile ("" : "+g" (r), "+g" (s), "+g" (t), "+g" (u)
: : "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10",
"11", "12");
return r + s + t + u;
}
on s390x with -O2 {-mno-lra,-mlra} to test both reload and LRA, LRA ICEs on
it, reload just gives up, but again it ought to be reloadable.
> 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;
That wouldn't handle e.g.
asm volatile ("" : "=m,m" (b), "=r,r" (b) : "1,1" (b));
case.
Jakub
