[PATCH/MIPS] Split Loongson MultiMedia extensions Instructions (MMI) from loongson2f/3a
Maciej W. Rozycki
macro@mips.com
Fri Jul 13 19:27:00 GMT 2018
Hi Paul,
Thank you for your submission. Your patch looks mostly good to me,
except that it does not apply:
1 out of 1 hunk FAILED -- saving rejects to file gas/testsuite/gas/mips/loongson-3a.d.rej
1 out of 1 hunk FAILED -- saving rejects to file gas/testsuite/gas/mips/loongson-3a.s.rej
See below for specific issues with your change, really minor, however
since you need to resubmit anyway, I'll let you make the necessary updates
yourself.
Also I don't see any earlier changes from you in binutils and this change
is substantial enough to require a copyright arrangement with FSF before
it can be accepted. Do you have one in place?
Specific issues follow.
> This patch split Loongson MultiMedia extensions Instructions (MMI)
> from loongson2f/3a.
> The MMI instructions set has been implemented in many loongson
> processors. I'd like to split it
> from loongson2f/3a and give it a name MMI. Even though i know that the
> R5900 used the name MMI
> in mips-opc.c, I also prefer the name MMI for those instructions.
> There are a lot of software has optimized
> with MMI,like pixman[1], FFmpeg[2].
>
> [1]https://cgit.freedesktop.org/pixman/commit/?id=c78e986085
> [2]http://ffmpeg.org/pipermail/ffmpeg-cvslog/2015-July/091449.html
Do you intend this text to become the commit description in the
repository? If so, then please reformat it as a proper paragraph,
observing the 74-column limit and placing two spaces after full stops.
> Changelog:
> 2018-07-013 Chenhua Xu <paul.hua.gm@gmail.com>
2018-07-13 please (or whatever date that will be). Also please use
a leading tab with ChangeLog entries, to ensure consistent formatting
between the change description and actual ChangeLog file contents.
> (mips_cpu_info_table):Add ASE_MMI for loongs2f/3a
Missing space after `:' and a full stop at the end; also `loongson2f/3a'
I suppose.
> * testsuite/gas/mips/loongso2f.d: Move mmi test to ...
> * testsuite/gas/mips/loongso2f-mmi.d: Here.
> * testsuite/gas/mips/loongso2f.s: Move mmi test to ...
> * testsuite/gas/mips/loongso2f-mmi.s: Here.
> * testsuite/gas/mips/loongso3a.d: Move mmi test to ...
> * testsuite/gas/mips/loongso3a-mmi.d: Here.
> * testsuite/gas/mips/loongso3a.s: Move mmi test to ...
> * testsuite/gas/mips/loongso3a-mmi.s: Here.
> * testsuite/gas/mips/mips.exp: involved
> loongso2f-mmi and loongso3a-mmi test.
Typos: s/loongso/loongson-/g. Also bad style and formatting in the last
line, perhaps:
* testsuite/gas/mips/mips.exp: Run loongson-2f-mmi and
loongson-3a-mmi tests.
?
> include/
> * elf/mips.h (AFL_ASE_MMI): New macros.
"New macro."
> * mips-opc.c(LMMI): New macro.
Missing space before the opening parenthesis.
> (mips_opcodes): Split mmi for loongson2f/3a.
Maybe:
(mips_opcodes): Replace IL2F|IL3A marking with LMMI for MMI
instructions.
?
> diff --git a/gas/doc/as.texi b/gas/doc/as.texi
> index 49b1ef1..b2afcd1 100644
> --- a/gas/doc/as.texi
> +++ b/gas/doc/as.texi
> @@ -1526,6 +1527,13 @@ Generate code for the Global INValidate (GINV) Application Specific
> Extension. This tells the assembler to accept GINV instructions.
> @samp{-mno-ginv} turns off this option.
>
> +@item -mmmi
> +@itemx -mno-mmi
> +Generate code for the Loongson MultiMedia extensions Instructions (MMI)
> +Application Specific Extension. This tells the assembler to accept MMI
Two spaces after a full stop please.
> diff --git a/gas/doc/c-mips.texi b/gas/doc/c-mips.texi
> index 24f6843..f01024a 100644
> --- a/gas/doc/c-mips.texi
> +++ b/gas/doc/c-mips.texi
> @@ -246,6 +246,13 @@ Generate code for the Global INValidate (GINV) Application Specific
> Extension. This tells the assembler to accept GINV instructions.
> @samp{-mno-ginv} turns off this option.
>
> +@item -mmmi
> +@itemx -mno-mmi
> +Generate code for the Loongson MultiMedia extensions Instructions (MMI)
> +Application Specific Extension. This tells the assembler to accept MMI
Likewise.
> diff --git a/gas/testsuite/gas/mips/loongson-2f-mmi.d b/gas/testsuite/gas/mips/loongson-2f-mmi.d
> new file mode 100644
> index 0000000..daa312e
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/loongson-2f-mmi.d
> @@ -0,0 +1,70 @@
> +#as: -march=loongson2f -mabi=o64
> +#objdump: -M reg-names=numeric -M mmi -dr
> +#name: Loongson-2F mmi tests
> +
> +.*: file format .*
> +
> +Disassembly of section .text:
> +
> +[0-9a-f]+ <simd_insns>:
> +.*: 4b420802 packsshb \$f0,\$f1,\$f2
> +.*: 4b2520c2 packsswh \$f3,\$f4,\$f5
> +.*: 4b683982 packushb \$f6,\$f7,\$f8
> +.*: 4bcb5240 paddb \$f9,\$f10,\$f11
> +.*: 4b4e6b00 paddh \$f12,\$f13,\$f14
> +.*: 4b7183c0 paddw \$f15,\$f16,\$f17
> +.*: 4bf49c80 paddd \$f18,\$f19,\$f20
> +.*: 4b97b540 paddsb \$f21,\$f22,\$f23
> +.*: 4b1ace00 paddsh \$f24,\$f25,\$f26
> +.*: 4bbde6c0 paddusb \$f27,\$f28,\$f29
> +.*: 4b220800 paddush \$f0,\$f1,\$f2
> +.*: 4be520c2 pandn \$f3,\$f4,\$f5
> +.*: 4b283988 pavgb \$f6,\$f7,\$f8
> +.*: 4b0b5248 pavgh \$f9,\$f10,\$f11
> +.*: 4b8e6b09 pcmpeqb \$f12,\$f13,\$f14
> +.*: 4b5183c9 pcmpeqh \$f15,\$f16,\$f17
> +.*: 4b149c89 pcmpeqw \$f18,\$f19,\$f20
> +.*: 4bb7b549 pcmpgtb \$f21,\$f22,\$f23
> +.*: 4b7ace09 pcmpgth \$f24,\$f25,\$f26
> +.*: 4b3de6c9 pcmpgtw \$f27,\$f28,\$f29
> +.*: 4b42080e pextrh \$f0,\$f1,\$f2
> +.*: 4b8520c3 pinsrh_0 \$f3,\$f4,\$f5
> +.*: 4ba83983 pinsrh_1 \$f6,\$f7,\$f8
> +.*: 4bcb5243 pinsrh_2 \$f9,\$f10,\$f11
> +.*: 4bee6b03 pinsrh_3 \$f12,\$f13,\$f14
> +.*: 4b7183ce pmaddhw \$f15,\$f16,\$f17
> +.*: 4b549c88 pmaxsh \$f18,\$f19,\$f20
> +.*: 4b97b548 pmaxub \$f21,\$f22,\$f23
> +.*: 4b7ace08 pminsh \$f24,\$f25,\$f26
> +.*: 4bbde6c8 pminub \$f27,\$f28,\$f29
> +.*: 4ba0080f pmovmskb \$f0,\$f1
> +.*: 4ba4188a pmulhuh \$f2,\$f3,\$f4
> +.*: 4b67314a pmulhh \$f5,\$f6,\$f7
> +.*: 4b4a4a0a pmullh \$f8,\$f9,\$f10
> +.*: 4b8d62ca pmuluw \$f11,\$f12,\$f13
> +.*: 4b307b8d pasubub \$f14,\$f15,\$f16
> +.*: 4b80944f biadd \$f17,\$f18
> +.*: 4b15a4c2 pshufh \$f19,\$f20,\$f21
> +.*: 4b38bd8a psllh \$f22,\$f23,\$f24
> +.*: 4b1bd64a psllw \$f25,\$f26,\$f27
> +.*: 4b7eef0b psrah \$f28,\$f29,\$f30
> +.*: 4b42080b psraw \$f0,\$f1,\$f2
> +.*: 4b2520cb psrlh \$f3,\$f4,\$f5
> +.*: 4b08398b psrlw \$f6,\$f7,\$f8
> +.*: 4bcb5241 psubb \$f9,\$f10,\$f11
> +.*: 4b4e6b01 psubh \$f12,\$f13,\$f14
> +.*: 4b7183c1 psubw \$f15,\$f16,\$f17
> +.*: 4bf49c81 psubd \$f18,\$f19,\$f20
> +.*: 4b97b541 psubsb \$f21,\$f22,\$f23
> +.*: 4b1ace01 psubsh \$f24,\$f25,\$f26
> +.*: 4bbde6c1 psubusb \$f27,\$f28,\$f29
> +.*: 4b220801 psubush \$f0,\$f1,\$f2
> +.*: 4b6520c3 punpckhbh \$f3,\$f4,\$f5
> +.*: 4b283983 punpckhhw \$f6,\$f7,\$f8
> +.*: 4bab524b punpckhwd \$f9,\$f10,\$f11
> +.*: 4b4e6b03 punpcklbh \$f12,\$f13,\$f14
> +.*: 4b1183c3 punpcklhw \$f15,\$f16,\$f17
> +.*: 4b949c8b punpcklwd \$f18,\$f19,\$f20
> +#pass
> +
> +
Trailing empty lines.
> diff --git a/gas/testsuite/gas/mips/loongson-2f-mmi.s b/gas/testsuite/gas/mips/loongson-2f-mmi.s
> new file mode 100644
> index 0000000..baba203
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/loongson-2f-mmi.s
> @@ -0,0 +1,63 @@
> + .text
> + .set noreorder
> +
> +simd_insns:
> + packsshb $f0, $f1, $f2
> + packsswh $f3, $f4, $f5
> + packushb $f6, $f7, $f8
> + paddb $f9, $f10, $f11
> + paddh $f12, $f13, $f14
> + paddw $f15, $f16, $f17
> + paddd $f18, $f19, $f20
> + paddsb $f21, $f22, $f23
> + paddsh $f24, $f25, $f26
> + paddusb $f27, $f28, $f29
> + paddush $f0, $f1, $f2
> + pandn $f3, $f4, $f5
> + pavgb $f6, $f7, $f8
> + pavgh $f9, $f10, $f11
> + pcmpeqb $f12, $f13, $f14
> + pcmpeqh $f15, $f16, $f17
> + pcmpeqw $f18, $f19, $f20
> + pcmpgtb $f21, $f22, $f23
> + pcmpgth $f24, $f25, $f26
> + pcmpgtw $f27, $f28, $f29
> + pextrh $f0, $f1, $f2
> + pinsrh_0 $f3, $f4, $f5
> + pinsrh_1 $f6, $f7, $f8
> + pinsrh_2 $f9, $f10, $f11
> + pinsrh_3 $f12, $f13, $f14
> + pmaddhw $f15, $f16, $f17
> + pmaxsh $f18, $f19, $f20
> + pmaxub $f21, $f22, $f23
> + pminsh $f24, $f25, $f26
> + pminub $f27, $f28, $f29
> + pmovmskb $f0, $f1
> + pmulhuh $f2, $f3, $f4
> + pmulhh $f5, $f6, $f7
> + pmullh $f8, $f9, $f10
> + pmuluw $f11, $f12, $f13
> + pasubub $f14, $f15, $f16
> + biadd $f17, $f18
> + pshufh $f19, $f20, $f21
> + psllh $f22, $f23, $f24
> + psllw $f25, $f26, $f27
> + psrah $f28, $f29, $f30
> + psraw $f0, $f1, $f2
> + psrlh $f3, $f4, $f5
> + psrlw $f6, $f7, $f8
> + psubb $f9, $f10, $f11
> + psubh $f12, $f13, $f14
> + psubw $f15, $f16, $f17
> + psubd $f18, $f19, $f20
> + psubsb $f21, $f22, $f23
> + psubsh $f24, $f25, $f26
> + psubusb $f27, $f28, $f29
> + psubush $f0, $f1, $f2
> + punpckhbh $f3, $f4, $f5
> + punpckhhw $f6, $f7, $f8
> + punpckhwd $f9, $f10, $f11
> + punpcklbh $f12, $f13, $f14
> + punpcklhw $f15, $f16, $f17
> + punpcklwd $f18, $f19, $f20
> +
Likewise.
> diff --git a/gas/testsuite/gas/mips/loongson-3a-mmi.d b/gas/testsuite/gas/mips/loongson-3a-mmi.d
> new file mode 100644
> index 0000000..9a03154
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/loongson-3a-mmi.d
> @@ -0,0 +1,70 @@
> +#as: -march=loongson3a -mabi=o64
> +#objdump: -M reg-names=numeric -M mmi -dr
> +#name: Loongson-3A mmi tests
> +
> +.*: file format .*
> +
> +Disassembly of section .text:
> +
> +[0-9a-f]+ <simd_insns>:
> +.*: 4b420802 packsshb \$f0,\$f1,\$f2
> +.*: 4b2520c2 packsswh \$f3,\$f4,\$f5
> +.*: 4b683982 packushb \$f6,\$f7,\$f8
> +.*: 4bcb5240 paddb \$f9,\$f10,\$f11
> +.*: 4b4e6b00 paddh \$f12,\$f13,\$f14
> +.*: 4b7183c0 paddw \$f15,\$f16,\$f17
> +.*: 4bf49c80 paddd \$f18,\$f19,\$f20
> +.*: 4b97b540 paddsb \$f21,\$f22,\$f23
> +.*: 4b1ace00 paddsh \$f24,\$f25,\$f26
> +.*: 4bbde6c0 paddusb \$f27,\$f28,\$f29
> +.*: 4b220800 paddush \$f0,\$f1,\$f2
> +.*: 4be520c2 pandn \$f3,\$f4,\$f5
> +.*: 4b283988 pavgb \$f6,\$f7,\$f8
> +.*: 4b0b5248 pavgh \$f9,\$f10,\$f11
> +.*: 4b8e6b09 pcmpeqb \$f12,\$f13,\$f14
> +.*: 4b5183c9 pcmpeqh \$f15,\$f16,\$f17
> +.*: 4b149c89 pcmpeqw \$f18,\$f19,\$f20
> +.*: 4bb7b549 pcmpgtb \$f21,\$f22,\$f23
> +.*: 4b7ace09 pcmpgth \$f24,\$f25,\$f26
> +.*: 4b3de6c9 pcmpgtw \$f27,\$f28,\$f29
> +.*: 4b42080e pextrh \$f0,\$f1,\$f2
> +.*: 4b8520c3 pinsrh_0 \$f3,\$f4,\$f5
> +.*: 4ba83983 pinsrh_1 \$f6,\$f7,\$f8
> +.*: 4bcb5243 pinsrh_2 \$f9,\$f10,\$f11
> +.*: 4bee6b03 pinsrh_3 \$f12,\$f13,\$f14
> +.*: 4b7183ce pmaddhw \$f15,\$f16,\$f17
> +.*: 4b549c88 pmaxsh \$f18,\$f19,\$f20
> +.*: 4b97b548 pmaxub \$f21,\$f22,\$f23
> +.*: 4b7ace08 pminsh \$f24,\$f25,\$f26
> +.*: 4bbde6c8 pminub \$f27,\$f28,\$f29
> +.*: 4ba0080f pmovmskb \$f0,\$f1
> +.*: 4ba4188a pmulhuh \$f2,\$f3,\$f4
> +.*: 4b67314a pmulhh \$f5,\$f6,\$f7
> +.*: 4b4a4a0a pmullh \$f8,\$f9,\$f10
> +.*: 4b8d62ca pmuluw \$f11,\$f12,\$f13
> +.*: 4b307b8d pasubub \$f14,\$f15,\$f16
> +.*: 4b80944f biadd \$f17,\$f18
> +.*: 4b15a4c2 pshufh \$f19,\$f20,\$f21
> +.*: 4b38bd8a psllh \$f22,\$f23,\$f24
> +.*: 4b1bd64a psllw \$f25,\$f26,\$f27
> +.*: 4b7eef0b psrah \$f28,\$f29,\$f30
> +.*: 4b42080b psraw \$f0,\$f1,\$f2
> +.*: 4b2520cb psrlh \$f3,\$f4,\$f5
> +.*: 4b08398b psrlw \$f6,\$f7,\$f8
> +.*: 4bcb5241 psubb \$f9,\$f10,\$f11
> +.*: 4b4e6b01 psubh \$f12,\$f13,\$f14
> +.*: 4b7183c1 psubw \$f15,\$f16,\$f17
> +.*: 4bf49c81 psubd \$f18,\$f19,\$f20
> +.*: 4b97b541 psubsb \$f21,\$f22,\$f23
> +.*: 4b1ace01 psubsh \$f24,\$f25,\$f26
> +.*: 4bbde6c1 psubusb \$f27,\$f28,\$f29
> +.*: 4b220801 psubush \$f0,\$f1,\$f2
> +.*: 4b6520c3 punpckhbh \$f3,\$f4,\$f5
> +.*: 4b283983 punpckhhw \$f6,\$f7,\$f8
> +.*: 4bab524b punpckhwd \$f9,\$f10,\$f11
> +.*: 4b4e6b03 punpcklbh \$f12,\$f13,\$f14
> +.*: 4b1183c3 punpcklhw \$f15,\$f16,\$f17
> +.*: 4b949c8b punpcklwd \$f18,\$f19,\$f20
> +#pass
> +
> +
Likewise.
> diff --git a/gas/testsuite/gas/mips/loongson-3a-mmi.s b/gas/testsuite/gas/mips/loongson-3a-mmi.s
> new file mode 100644
> index 0000000..baba203
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/loongson-3a-mmi.s
> @@ -0,0 +1,63 @@
> + .text
> + .set noreorder
> +
> +simd_insns:
> + packsshb $f0, $f1, $f2
> + packsswh $f3, $f4, $f5
> + packushb $f6, $f7, $f8
> + paddb $f9, $f10, $f11
> + paddh $f12, $f13, $f14
> + paddw $f15, $f16, $f17
> + paddd $f18, $f19, $f20
> + paddsb $f21, $f22, $f23
> + paddsh $f24, $f25, $f26
> + paddusb $f27, $f28, $f29
> + paddush $f0, $f1, $f2
> + pandn $f3, $f4, $f5
> + pavgb $f6, $f7, $f8
> + pavgh $f9, $f10, $f11
> + pcmpeqb $f12, $f13, $f14
> + pcmpeqh $f15, $f16, $f17
> + pcmpeqw $f18, $f19, $f20
> + pcmpgtb $f21, $f22, $f23
> + pcmpgth $f24, $f25, $f26
> + pcmpgtw $f27, $f28, $f29
> + pextrh $f0, $f1, $f2
> + pinsrh_0 $f3, $f4, $f5
> + pinsrh_1 $f6, $f7, $f8
> + pinsrh_2 $f9, $f10, $f11
> + pinsrh_3 $f12, $f13, $f14
> + pmaddhw $f15, $f16, $f17
> + pmaxsh $f18, $f19, $f20
> + pmaxub $f21, $f22, $f23
> + pminsh $f24, $f25, $f26
> + pminub $f27, $f28, $f29
> + pmovmskb $f0, $f1
> + pmulhuh $f2, $f3, $f4
> + pmulhh $f5, $f6, $f7
> + pmullh $f8, $f9, $f10
> + pmuluw $f11, $f12, $f13
> + pasubub $f14, $f15, $f16
> + biadd $f17, $f18
> + pshufh $f19, $f20, $f21
> + psllh $f22, $f23, $f24
> + psllw $f25, $f26, $f27
> + psrah $f28, $f29, $f30
> + psraw $f0, $f1, $f2
> + psrlh $f3, $f4, $f5
> + psrlw $f6, $f7, $f8
> + psubb $f9, $f10, $f11
> + psubh $f12, $f13, $f14
> + psubw $f15, $f16, $f17
> + psubd $f18, $f19, $f20
> + psubsb $f21, $f22, $f23
> + psubsh $f24, $f25, $f26
> + psubusb $f27, $f28, $f29
> + psubush $f0, $f1, $f2
> + punpckhbh $f3, $f4, $f5
> + punpckhhw $f6, $f7, $f8
> + punpckhwd $f9, $f10, $f11
> + punpcklbh $f12, $f13, $f14
> + punpcklhw $f15, $f16, $f17
> + punpcklwd $f18, $f19, $f20
> +
Likewise.
> diff --git a/include/opcode/mips.h b/include/opcode/mips.h
> index 1ab1780..1d0998c 100644
> --- a/include/opcode/mips.h
> +++ b/include/opcode/mips.h
> @@ -1302,6 +1302,8 @@ static const unsigned int mips_isa_table[] = {
> #define ASE_CRC64 0x00080000
> /* Global INValidate Extension. */
> #define ASE_GINV 0x00100000
> +/* Loongson MultiMedia extensions Instructions (MMI). */
Two spaces after a full stop please.
> +#define ASE_MMI 0x00200000
>
> /* MIPS ISA defines, use instead of hardcoding ISA level. */
>
> diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
> index bbf2132..b9fd9e5 100644
> --- a/opcodes/mips-dis.c
> +++ b/opcodes/mips-dis.c
> @@ -626,11 +626,11 @@ const struct mips_arch_choice mips_arch_choices[] =
> NULL, 0, mips_cp1_names_numeric, mips_hwr_names_numeric },
>
> { "loongson2f", 1, bfd_mach_mips_loongson_2f, CPU_LOONGSON_2F,
> - ISA_MIPS3 | INSN_LOONGSON_2F, 0, mips_cp0_names_numeric,
> + ISA_MIPS3 | ASE_MMI, 0, mips_cp0_names_numeric,
> NULL, 0, mips_cp1_names_numeric, mips_hwr_names_numeric },
>
> { "loongson3a", 1, bfd_mach_mips_loongson_3a, CPU_LOONGSON_3A,
> - ISA_MIPS64R2 | INSN_LOONGSON_3A, 0, mips_cp0_names_numeric,
> + ISA_MIPS64R2 | ASE_MMI, 0, mips_cp0_names_numeric,
> NULL, 0, mips_cp1_names_mips3264, mips_hwr_names_numeric },
Why do you remove INSN_LOONGSON_2F and INSN_LOONGSON_3A flags? AFAICT
the MMI instructions are not the only ones these processors have?
> diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c
> index 1cbcbc6..0dc15f9 100644
> --- a/opcodes/mips-opc.c
> +++ b/opcodes/mips-opc.c
> @@ -412,6 +412,9 @@ decode_mips_operand (const char *p)
> /* Global INValidate (GINV) support. */
> #define GINV ASE_GINV
>
> +/* Loongson MultiMedia extensions Instructions (MMI) support. */
Two spaces after a full stop please.
Please resubmit with these issues addressed, and, as noted at the top,
explain your copyright status with FSF.
Maciej
More information about the Binutils
mailing list