"Moore, Catherine" <[email protected]> writes:
>> -----Original Message-----
>> From: Richard Sandiford [mailto:[email protected]]
>> Sent: Tuesday, April 15, 2014 4:32 PM
>> To: Moore, Catherine
>> Cc: Rozycki, Maciej; Matthew Fortune; [email protected]
>> Subject: Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and
>> SB16
>>
>> "Moore, Catherine" <[email protected]> writes:
>> >> -----Original Message-----
>> >> From: Moore, Catherine
>> >> Sent: Tuesday, April 15, 2014 8:49 AM
>> >> To: Rozycki, Maciej; Richard Sandiford
>> >> Cc: Matthew Fortune; [email protected]; Moore, Catherine
>> >> Subject: RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and
>> >> SB16
>> >>
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: Maciej W. Rozycki [mailto:[email protected]]
>> >> > Sent: Tuesday, April 15, 2014 7:28 AM
>> >> > To: Richard Sandiford
>> >> > Cc: Matthew Fortune; Moore, Catherine; [email protected]
>> >> > Subject: Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16
>> >> > and
>> >> > SB16
>> >> >
>> >> > On Tue, 15 Apr 2014, Richard Sandiford wrote:
>> >> >
>> >> > > > I believe you need to adjust constraints to ensure constant 0
>> >> > > > is known to produce a 16-bit instruction encoding where possible.
>> >> > > > Otherwise you'll end up with suboptimal code when the
>> >> > > > instruction is in a branch delay slot.
>> >> > >
>> >> > > Yeah, it'd be good to do that too (although this is a preexisting
>> >> > > problem).
>> >> >
>> >> > Well, it depends on how you look at the problem being solved here
>> >> > -- if it is "for SW16, SH16 and SB16 GCC produces broken code for
>> >> > the `s0'
>> >> > source register", then indeed it is, whereas if it is "GCC does not
>> >> > handle the source register set for SW16, SH16 and SB16 correctly",
>> >> > then it is a part of the same problem, not completely corrected. I
>> >> > can live with that until 4.10/4.9.1 though if you prefer.
>> >> >
>> >> > > I'm relying on you guys to do the microMIPS stuff though -- I
>> >> > > don't have a way of testing it.
>> >> >
>> >> > An assembly/objdump test is enough to cover this, so you've got
>> >> > all tools at hand, although I understand you may not be inclined to
>> >> > rush working on it. ;)
>> >> >
>> >> I'll take care of this bit.
>> >
>> > I've attached an updated patch to address Maciej's concern with $0 and
>> > the microMIPS store instructions.
>> > Does this look okay to install?
>>
>> No, the point was that zero is modelled as a constant in RTL, so like Maciej
>> says, the way to handle it is to use the "J" constraint (like some of the
>> existing contraints use "dJ" for "any GPR or zero").
>>
>> What we want to test is that:
>>
>> *ptr = 0;
>>
>> is a 16-bit instruction. You could do that by adding "-dp" to the options
>> and
>> matching something like:
>>
>> MICROMIPS void
>> f1 (unsigned char *ptr)
>> {
>> *ptr = 0;
>> }
>>
>> ...[similarly for short and int]...
>>
>> /* { dg-final { scan-assembler "\tsb\t\\\$0, 0\\(\\\$4\\)\[^\n\]length
>> = 2" } }
Oops, I see I forgot the "*", should have:
\[^\n\]*length. But:
>> */ ...[similarly for sh and sw]...
>>
>> Completely untested. I bet the regexp needs different backslashes. :-)
>>
> Okay, this patch modifies the constraints instead. Okay?
>
>
>
> Index: testsuite/gcc.target/mips/umips-store16-2.c
> ===================================================================
> --- testsuite/gcc.target/mips/umips-store16-2.c (revision 0)
> +++ testsuite/gcc.target/mips/umips-store16-2.c (revision 0)
> @@ -0,0 +1,22 @@
> +/* { dg-options "(-mmicromips) -dp" } */
> +
> +MICROMIPS void
> +f1 (unsigned char *ptr)
> +{
> + *ptr = 0;
> +}
> +
> +MICROMIPS void
> +f2 (unsigned short *ptr)
> +{
> + *ptr = 0;
> +}
> +
> +MICROMIPS void
> +f3 (unsigned int *ptr)
> +{
> + *ptr = 0;
> +}
> +/* { dg-final { scan-assembler "\tsb\t\\\$0,0\\(\\\$\[0-9\]+\\).*length = 2"
> } } */
...it does need to be \[^\n\], since "." can match newlines in Tcl.
OK with that change if the new tests still pass, and if a full test run
passes with -mmicromips.
Thanks,
Richard