| Message ID | alpine.DEB.1.10.1411171635480.2881@tp.orcam.me.uk |
|---|---|
| State | Accepted |
| Headers | show |
Hi Maciej, > -----Original Message----- > From: Rozycki, Maciej > Sent: Monday, November 17, 2014 2:39 PM > To: gcc-patches@gcc.gnu.org > Cc: Moore, Catherine; Eric Christopher; Matthew Fortune > Subject: [PATCH] microMIPS/GCC: Correct 64-bit instruction sizes > > Hi, > > Noticed in an attempt to build a 64-bit microMIPS Linux kernel. > > None of the 64-bit operations have a short instruction encoding in the > microMIPS instruction set. Despite that our code has been written such that > the presence of a short encoding is assumed for both 32-bit and 64-bit > operations. This leads to assembly warnings like: > > {standard input}: Assembler messages: > {standard input}:146: Warning: wrong size instruction in a 16-bit branch delay > slot {standard input}:544: Warning: wrong size instruction in a 16-bit branch > delay slot {standard input}:1071: Warning: wrong size instruction in a 16-bit > branch delay slot {standard input}:1088: Warning: wrong size instruction in a > 16-bit branch delay slot {standard input}:1833: Warning: wrong size > instruction in a 16-bit branch delay slot > > This is because code in question produced by GCC looks like: > > jals get_option # > daddiu $4,$sp,16 #,, > > (in a `.set noreorder' fragment) and there is no short encoding for the > DADDIU instruction available. > > At least two approaches are possible to address this problem, either by > splitting the iterated patterns affected into separate 32-bit and 64-bit > patterns, or by limiting the affected alternatives to 32-bit operations only. I > found the latter less intrusive, with the use of an extra `compression' > attribute value: `micromips32'. I prefer the second approach as well. This is implemented with the change below, > fixing the issues seen in 64-bit compilation. Code produced now looks like: > > jal get_option # > daddiu $4,$sp,16 #,, > > instead. > > Some operations are operand size agnostic, these include LI16, MOVE16, and > all the relevant logical instructions. These retain the `micromips' > setting for the `compression' attribute. MOVEP is also operand size agnostic, > but it must not be scheduled into a delay slot and it is therefore handled > separately, with no `compression' attribute defined. > > Regression-tested with the mips-linux-gnu target and these multilibs: > > -EB > -EB -mips16 > -EB -mmicromips > -EB -mabi=n32 > -EB -mabi=64 > > and the -EL variants of same, with no changes in results. I have also checked > that microMIPS shared glibc libraries have not changed when rebuilt with the > updated compiler. > > OK to apply? > > 2014-11-14 Maciej W. Rozycki <macro@codesourcery.com> > > gcc/ > * config/mips/mips.md (compression): Add `micromips32' setting. > (enabled, length): Handle it. > (shift_compression): Replace `micromips' with `micromips32' in > the `compression' attribute. > (*add<mode>3, sub<mode>3): Likewise. Yes, this looks good. Thanks, Catherine
On Mon, 17 Nov 2014, Moore, Catherine wrote: > > * config/mips/mips.md (compression): Add `micromips32' setting. > > (enabled, length): Handle it. > > (shift_compression): Replace `micromips' with `micromips32' in > > the `compression' attribute. > > (*add<mode>3, sub<mode>3): Likewise. > > Yes, this looks good. Applied now, thanks for your review. Maciej
Index: gcc-fsf-trunk-quilt/gcc/config/mips/mips.md =================================================================== --- gcc-fsf-trunk-quilt.orig/gcc/config/mips/mips.md 2014-11-14 18:34:25.000000000 +0000 +++ gcc-fsf-trunk-quilt/gcc/config/mips/mips.md 2014-11-14 20:56:20.278971795 +0000 @@ -429,7 +429,7 @@ (const_string "yes") (const_string "no"))) -(define_attr "compression" "none,all,micromips" +(define_attr "compression" "none,all,micromips32,micromips" (const_string "none")) (define_attr "enabled" "no,yes" @@ -440,7 +440,7 @@ || TARGET_O32_FP64A_ABI") (eq_attr "dword_mode" "yes")) (const_string "no") - (and (eq_attr "compression" "micromips") + (and (eq_attr "compression" "micromips32,micromips") (match_test "!TARGET_MICROMIPS")) (const_string "no")] (const_string "yes"))) @@ -526,7 +526,9 @@ ;; but there are special cases for branches (which must be handled here) ;; and for compressed single instructions. (define_attr "length" "" - (cond [(and (eq_attr "compression" "micromips,all") + (cond [(and (ior (eq_attr "compression" "micromips,all") + (and (eq_attr "compression" "micromips32") + (eq_attr "mode" "SI,SF"))) (eq_attr "dword_mode" "no") (match_test "TARGET_MICROMIPS")) (const_int 2) @@ -979,8 +981,8 @@ (xor "xori") (and "andi")]) -(define_code_attr shift_compression [(ashift "micromips") - (lshiftrt "micromips") +(define_code_attr shift_compression [(ashift "micromips32") + (lshiftrt "micromips32") (ashiftrt "none")]) ;; <fcond> is the c.cond.fmt condition associated with a particular code. @@ -1163,7 +1165,7 @@ return "<d>addiu\t%0,%1,%2"; } [(set_attr "alu_type" "add") - (set_attr "compression" "micromips,*,micromips,micromips,micromips,micromips,*") + (set_attr "compression" "micromips32,*,micromips32,micromips32,micromips32,micromips32,*") (set_attr "mode" "<MODE>")]) (define_insn "*add<mode>3_mips16" @@ -1381,7 +1383,7 @@ "" "<d>subu\t%0,%1,%2" [(set_attr "alu_type" "sub") - (set_attr "compression" "micromips,*") + (set_attr "compression" "micromips32,*") (set_attr "mode" "<MODE>")]) (define_insn "*subsi3_extended"
Hi, Noticed in an attempt to build a 64-bit microMIPS Linux kernel. None of the 64-bit operations have a short instruction encoding in the microMIPS instruction set. Despite that our code has been written such that the presence of a short encoding is assumed for both 32-bit and 64-bit operations. This leads to assembly warnings like: {standard input}: Assembler messages: {standard input}:146: Warning: wrong size instruction in a 16-bit branch delay slot {standard input}:544: Warning: wrong size instruction in a 16-bit branch delay slot {standard input}:1071: Warning: wrong size instruction in a 16-bit branch delay slot {standard input}:1088: Warning: wrong size instruction in a 16-bit branch delay slot {standard input}:1833: Warning: wrong size instruction in a 16-bit branch delay slot This is because code in question produced by GCC looks like: jals get_option # daddiu $4,$sp,16 #,, (in a `.set noreorder' fragment) and there is no short encoding for the DADDIU instruction available. At least two approaches are possible to address this problem, either by splitting the iterated patterns affected into separate 32-bit and 64-bit patterns, or by limiting the affected alternatives to 32-bit operations only. I found the latter less intrusive, with the use of an extra `compression' attribute value: `micromips32'. This is implemented with the change below, fixing the issues seen in 64-bit compilation. Code produced now looks like: jal get_option # daddiu $4,$sp,16 #,, instead. Some operations are operand size agnostic, these include LI16, MOVE16, and all the relevant logical instructions. These retain the `micromips' setting for the `compression' attribute. MOVEP is also operand size agnostic, but it must not be scheduled into a delay slot and it is therefore handled separately, with no `compression' attribute defined. Regression-tested with the mips-linux-gnu target and these multilibs: -EB -EB -mips16 -EB -mmicromips -EB -mabi=n32 -EB -mabi=64 and the -EL variants of same, with no changes in results. I have also checked that microMIPS shared glibc libraries have not changed when rebuilt with the updated compiler. OK to apply? 2014-11-14 Maciej W. Rozycki <macro@codesourcery.com> gcc/ * config/mips/mips.md (compression): Add `micromips32' setting. (enabled, length): Handle it. (shift_compression): Replace `micromips' with `micromips32' in the `compression' attribute. (*add<mode>3, sub<mode>3): Likewise. Maciej gcc-umips32-compression.diff