"Moore, Catherine" <catherine_mo...@mentor.com> writes: >> -----Original Message----- >> From: Richard Sandiford [mailto:rdsandif...@googlemail.com] >> Sent: Tuesday, April 15, 2014 4:32 PM >> To: Moore, Catherine >> Cc: Rozycki, Maciej; Matthew Fortune; gcc-patches@gcc.gnu.org >> Subject: Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and >> SB16 >> >> "Moore, Catherine" <catherine_mo...@mentor.com> writes: >> >> -----Original Message----- >> >> From: Moore, Catherine >> >> Sent: Tuesday, April 15, 2014 8:49 AM >> >> To: Rozycki, Maciej; Richard Sandiford >> >> Cc: Matthew Fortune; gcc-patches@gcc.gnu.org; Moore, Catherine >> >> Subject: RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and >> >> SB16 >> >> >> >> >> >> >> >> > -----Original Message----- >> >> > From: Maciej W. Rozycki [mailto:ma...@codesourcery.com] >> >> > Sent: Tuesday, April 15, 2014 7:28 AM >> >> > To: Richard Sandiford >> >> > Cc: Matthew Fortune; Moore, Catherine; gcc-patches@gcc.gnu.org >> >> > 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