RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
-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 }
}
*/ ...[similarly for sh and sw]...
Completely untested. I bet the regexp needs different backslashes. :-)
Okay, this patch modifies the constraints instead. Okay?
umips-store.cl
Description: umips-store.cl
umips-store.patch
Description: umips-store.patch
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
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
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Maciej W. Rozycki ma...@codesourcery.com writes: On Sat, 12 Apr 2014, Richard Sandiford wrote: I went ahead and applied the adjusted version of the patch to trunk as below (because I wanted to add a testcase too). 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). I'm relying on you guys to do the microMIPS stuff though -- I don't have a way of testing it. Thanks, Richard
RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Richard Sandiford rdsandif...@googlemail.com writes: Maciej W. Rozycki ma...@codesourcery.com writes: On Sat, 12 Apr 2014, Richard Sandiford wrote: I went ahead and applied the adjusted version of the patch to trunk as below (because I wanted to add a testcase too). 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). I'm relying on you guys to do the microMIPS stuff though -- I don't have a way of testing it. FYI, we have GNUSIM patches awaiting submission to add micromips support. We are waiting on copyright assignment for GDB which is why they are not available yet. We were planning on getting them submitting as 'on behalf of' but it seems this may not be permitted any more by FSF. Matthew
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. ;) Maciej
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.
RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
-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? Thanks, Catherine umips-zero.cl Description: umips-zero.cl umips-zero.patch Description: umips-zero.patch
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 } }
*/
...[similarly for sh and sw]...
Completely untested. I bet the regexp needs different backslashes. :-)
Thanks,
Richard
RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
-Original Message-
From: Richard Sandiford [mailto:rdsandif...@googlemail.com]
Sent: Saturday, April 12, 2014 6:41 AM
To: Matthew Fortune
Cc: Moore, Catherine; gcc-patches@gcc.gnu.org; rguent...@suse.de;
ja...@redhat.com
Subject: Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and
SB16
Richard Sandiford rdsandif...@googlemail.com writes:
Matthew Fortune matthew.fort...@imgtec.com writes:
Hi Catherine/Richard,
I think there may be some impact on register move costs by
introducing this class.
Yeah, I was worried about that too. I'm going to do some code
comparison tests for SE and MIPS16 to see what happens.
OK, I compared the .s testsuite output for -O, -O2, -O3, -Os, -O -mips16 -
mabi=32 and -Os -mips16 -mabi=32 on mips64-linux-gnu.
I also tried CSiBE with -O2 and -Os -mips16 -mabi=32. The code was
identical in all cases so I think we should be OK.
I went ahead and applied the adjusted version of the patch to trunk as below
(because I wanted to add a testcase too).
Thanks for doing this.
Adding a new register class is definitely a bit invasive for this stage of
4.9.
OTOH microMIPS is a new feature and it would be good to have it working in
4.9.0. Since the testing suggests that the patch really doesn't affect non-
microMIPS code, I'd like it to go in 4.9 now.
Richard, Jakub, would that be OK?
Okay, understood. Will wait to hear.
gcc/
2014-04-12 Catherine Moore c...@codesourcery.com
* config/mips/constraints.md: Add new register constraint kb.
* config/mips/mips.md (*movmode_internal): Use constraint
kb.
(*movhi_internal): Likewise.
(*movqi_internal): Likewise.
* config/mips/mips.h (M16_STORE_REGS): New register class.
(REG_CLASS_NAMES): Add M16_STORE_REGS.
(REG_CLASS_CONTENTS): Likewise.
* config/mips/mips.c (mips_regno_to_class): Add
M16_STORE_REGS.
gcc/testsuite/
* gcc.target/mips/umips-store16-1.c: New test.
Index: gcc/config/mips/constraints.md
==
=
--- gcc/config/mips/constraints.md2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/constraints.md2014-04-12 10:38:48.895224932 +0100
@@ -92,6 +92,9 @@ (define_register_constraint D COP3_RE ;; but the
DSP version allows any accumulator target.
(define_register_constraint ka ISA_HAS_DSP_MULT ? ACC_REGS :
MD_REGS)
+(define_register_constraint kb M16_STORE_REGS
+ @internal)
+
(define_constraint kf
@internal
(match_operand 0 force_to_mem_operand))
Index: gcc/config/mips/mips.md
==
=
--- gcc/config/mips/mips.md 2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/mips.md 2014-04-12 10:38:48.925225200 +0100
@@ -4437,7 +4437,7 @@ (define_expand movmode
(define_insn *movmode_internal
[(set (match_operand:IMOVE32 0 nonimmediate_operand
=d,!u,!u,d,e,!u,!ks,d,ZS,ZT,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,
*m)
- (match_operand:IMOVE32 1 move_operand
d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!u,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C
*D,*B*C*D))]
+ (match_operand:IMOVE32 1 move_operand
+d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!kb,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B
+*C*D,*B*C*D))]
!TARGET_MIPS16
(register_operand (operands[0], MODEmode)
|| reg_or_0_operand (operands[1], MODEmode))
@@ -4578,7 +4578,7 @@ (define_expand movhi
(define_insn *movhi_internal
[(set (match_operand:HI 0 nonimmediate_operand
=d,!u,d,!u,d,ZU,m,*a,*d)
- (match_operand:HI 1 move_operand
d,J,I,ZU,m,!u,dJ,*d*J,*a))]
+ (match_operand:HI 1 move_operand
d,J,I,ZU,m,!kb,dJ,*d*J,*a))]
!TARGET_MIPS16
(register_operand (operands[0], HImode)
|| reg_or_0_operand (operands[1], HImode))
@@ -4654,7 +4654,7 @@ (define_expand movqi
(define_insn *movqi_internal
[(set (match_operand:QI 0 nonimmediate_operand
=d,!u,d,!u,d,ZV,m,*a,*d)
- (match_operand:QI 1 move_operand
d,J,I,ZW,m,!u,dJ,*d*J,*a))]
+ (match_operand:QI 1 move_operand
d,J,I,ZW,m,!kb,dJ,*d*J,*a))]
!TARGET_MIPS16
(register_operand (operands[0], QImode)
|| reg_or_0_operand (operands[1], QImode))
Index: gcc/config/mips/mips.h
==
=
--- gcc/config/mips/mips.h2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/mips.h2014-04-12 10:38:48.924225191 +0100
@@ -1870,6 +1870,7 @@ #define PIC_OFFSET_TABLE_REGNUM \ enum
reg_class {
NO_REGS, /* no registers in set */
+ M16_STORE_REGS,/* microMIPS store registers */
M16_REGS, /* mips16 directly accessible registers */
T_REG, /* mips16 T register ($24) */
M16_T_REGS,/* mips16 registers plus T register */
@@ -1907,6 +1908,7 @@ #define GENERAL_REGS GR_REGS
#define REG_CLASS_NAMES
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Moore, Catherine catherine_mo...@mentor.com writes: Adding a new register class is definitely a bit invasive for this stage of 4.9. OTOH microMIPS is a new feature and it would be good to have it working in 4.9.0. Since the testing suggests that the patch really doesn't affect non- microMIPS code, I'd like it to go in 4.9 now. Richard, Jakub, would that be OK? Okay, understood. Will wait to hear. The decision on IRC was that this is too invasive for 4.9.0 at this stage. It would probably be going in without another release candidate before the release. Thanks, Richard
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
On Sat, 12 Apr 2014, Richard Sandiford wrote: I went ahead and applied the adjusted version of the patch to trunk as below (because I wanted to add a testcase too). 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. E.g. here: --- gcc/config/mips/mips.md 2014-04-12 10:36:09.105788710 +0100 +++ gcc/config/mips/mips.md 2014-04-12 10:38:48.925225200 +0100 @@ -4437,7 +4437,7 @@ (define_expand movmode (define_insn *movmode_internal [(set (match_operand:IMOVE32 0 nonimmediate_operand =d,!u,!u,d,e,!u,!ks,d,ZS,ZT,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,*m) - (match_operand:IMOVE32 1 move_operand d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!u,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D))] + (match_operand:IMOVE32 1 move_operand d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!kb,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D))] !TARGET_MIPS16 (register_operand (operands[0], MODEmode) || reg_or_0_operand (operands[1], MODEmode)) using: (match_operand:IMOVE32 1 move_operand d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!kbJ,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D))] will make: jalsfoo sw $0, 0($2) be produced (6 bytes) rather than: jal foo sw $0, 0($2) (8 bytes), because the !kbJ/ZT constraint pair will match rather than more general dJ/m. Likewise with the other two RTL patterns. I'm fairly sure you'll be able to come up with an appropriate test case to cover it (or to prove me wrong if I missed something here). Maciej
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Matthew Fortune matthew.fort...@imgtec.com writes:
Hi Catherine/Richard,
I think there may be some impact on register move costs by introducing
this class.
Yeah, I was worried about that too. I'm going to do some code comparison
tests for SE and MIPS16 to see what happens.
Is it worth having mips_canonicalize_move_class return M16_REGS for
M16_STORE_REGS to reduce the effect on costings? Given the extra
register is only $0 then this would seem mostly acceptable albeit
slightly strange.
This will happen automatically, since M16_STORE_REGS is just M16_REGS
without $16. (M16_STORE_REGS doesn't include $0, since that's handled
as a constant instead.)
However:
Index: config/mips/mips.c
===
--- config/mips/mips.c (revision 209307)
+++ config/mips/mips.c (working copy)
@@ -648,7 +648,7 @@ static mips_one_only_stub *mips16_set_fcsr_stub;
/* Index R is the smallest register class that contains register R. */
const enum reg_class mips_regno_to_class[FIRST_PSEUDO_REGISTER] = {
- LEA_REGS,LEA_REGS, M16_REGS, V1_REG,
+ M16_STORE_REGS, LEA_REGS,M16_REGS, V1_REG,
M16_REGS,M16_REGS, M16_REGS, M16_REGS,
LEA_REGS,LEA_REGS, LEA_REGS, LEA_REGS,
LEA_REGS,LEA_REGS, LEA_REGS, LEA_REGS,
this bit isn't right. We should leave entry 0 alone and change the
M16_REGS entries for every register except $16.
Thanks,
Richard
Re: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Richard Sandiford rdsandif...@googlemail.com writes:
Matthew Fortune matthew.fort...@imgtec.com writes:
Hi Catherine/Richard,
I think there may be some impact on register move costs by introducing
this class.
Yeah, I was worried about that too. I'm going to do some code comparison
tests for SE and MIPS16 to see what happens.
OK, I compared the .s testsuite output for -O, -O2, -O3, -Os,
-O -mips16 -mabi=32 and -Os -mips16 -mabi=32 on mips64-linux-gnu.
I also tried CSiBE with -O2 and -Os -mips16 -mabi=32. The code was
identical in all cases so I think we should be OK.
I went ahead and applied the adjusted version of the patch to trunk
as below (because I wanted to add a testcase too).
Adding a new register class is definitely a bit invasive for this stage
of 4.9. OTOH microMIPS is a new feature and it would be good to have
it working in 4.9.0. Since the testing suggests that the patch really
doesn't affect non-microMIPS code, I'd like it to go in 4.9 now.
Richard, Jakub, would that be OK?
Thanks,
Richard
gcc/
2014-04-12 Catherine Moore c...@codesourcery.com
* config/mips/constraints.md: Add new register constraint kb.
* config/mips/mips.md (*movmode_internal): Use constraint kb.
(*movhi_internal): Likewise.
(*movqi_internal): Likewise.
* config/mips/mips.h (M16_STORE_REGS): New register class.
(REG_CLASS_NAMES): Add M16_STORE_REGS.
(REG_CLASS_CONTENTS): Likewise.
* config/mips/mips.c (mips_regno_to_class): Add M16_STORE_REGS.
gcc/testsuite/
* gcc.target/mips/umips-store16-1.c: New test.
Index: gcc/config/mips/constraints.md
===
--- gcc/config/mips/constraints.md 2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/constraints.md 2014-04-12 10:38:48.895224932 +0100
@@ -92,6 +92,9 @@ (define_register_constraint D COP3_RE
;; but the DSP version allows any accumulator target.
(define_register_constraint ka ISA_HAS_DSP_MULT ? ACC_REGS : MD_REGS)
+(define_register_constraint kb M16_STORE_REGS
+ @internal)
+
(define_constraint kf
@internal
(match_operand 0 force_to_mem_operand))
Index: gcc/config/mips/mips.md
===
--- gcc/config/mips/mips.md 2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/mips.md 2014-04-12 10:38:48.925225200 +0100
@@ -4437,7 +4437,7 @@ (define_expand movmode
(define_insn *movmode_internal
[(set (match_operand:IMOVE32 0 nonimmediate_operand
=d,!u,!u,d,e,!u,!ks,d,ZS,ZT,m,*f,*f,*d,*m,*d,*z,*a,*d,*B*C*D,*B*C*D,*d,*m)
- (match_operand:IMOVE32 1 move_operand
d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!u,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D))]
+ (match_operand:IMOVE32 1 move_operand
d,J,Udb7,Yd,Yf,ZT,ZS,m,!ks,!kb,dJ,*d*J,*m,*f,*f,*z,*d,*J*d,*a,*d,*m,*B*C*D,*B*C*D))]
!TARGET_MIPS16
(register_operand (operands[0], MODEmode)
|| reg_or_0_operand (operands[1], MODEmode))
@@ -4578,7 +4578,7 @@ (define_expand movhi
(define_insn *movhi_internal
[(set (match_operand:HI 0 nonimmediate_operand =d,!u,d,!u,d,ZU,m,*a,*d)
- (match_operand:HI 1 move_operand d,J,I,ZU,m,!u,dJ,*d*J,*a))]
+ (match_operand:HI 1 move_operand
d,J,I,ZU,m,!kb,dJ,*d*J,*a))]
!TARGET_MIPS16
(register_operand (operands[0], HImode)
|| reg_or_0_operand (operands[1], HImode))
@@ -4654,7 +4654,7 @@ (define_expand movqi
(define_insn *movqi_internal
[(set (match_operand:QI 0 nonimmediate_operand =d,!u,d,!u,d,ZV,m,*a,*d)
- (match_operand:QI 1 move_operand d,J,I,ZW,m,!u,dJ,*d*J,*a))]
+ (match_operand:QI 1 move_operand
d,J,I,ZW,m,!kb,dJ,*d*J,*a))]
!TARGET_MIPS16
(register_operand (operands[0], QImode)
|| reg_or_0_operand (operands[1], QImode))
Index: gcc/config/mips/mips.h
===
--- gcc/config/mips/mips.h 2014-04-12 10:36:09.105788710 +0100
+++ gcc/config/mips/mips.h 2014-04-12 10:38:48.924225191 +0100
@@ -1870,6 +1870,7 @@ #define PIC_OFFSET_TABLE_REGNUM \
enum reg_class
{
NO_REGS, /* no registers in set */
+ M16_STORE_REGS, /* microMIPS store registers */
M16_REGS,/* mips16 directly accessible registers */
T_REG, /* mips16 T register ($24) */
M16_T_REGS, /* mips16 registers plus T register */
@@ -1907,6 +1908,7 @@ #define GENERAL_REGS GR_REGS
#define REG_CLASS_NAMES
\
{ \
NO_REGS, \
+ M16_STORE_REGS,\
M16_REGS, \
T_REG, \
RE: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16
Hi Catherine/Richard, I think there may be some impact on register move costs by introducing this class. Is it worth having mips_canonicalize_move_class return M16_REGS for M16_STORE_REGS to reduce the effect on costings? Given the extra register is only $0 then this would seem mostly acceptable albeit slightly strange. What do you think? regards, Matthew -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Moore, Catherine Sent: 11 April 2014 19:25 To: gcc-patches@gcc.gnu.org Cc: Richard Sandiford Subject: [PATCH] [MIPS] Fix operands for microMIPS SW16, SH16 and SB16 Hi Richard, This patch fixes a problem with the SW16, SH16 and SB16 microMIPS instructions. GCC is incorrectly calculating the length of these instructions if $16 is used as the source operand. The incorrect length calculation can cause a 32-bit instruction to be placed in a short delay slot. The assembler does detect this and issues a warning. This patch changes the allowable registers for the short store instructions to $0, $2-$7, and $17. Okay to install? Okay to install for 4.9? Thanks, Catherine
