[PATCH][MIPS] CRC ASE support
Maciej W. Rozycki
macro@mips.com
Thu Dec 7 23:57:00 GMT 2017
Hi Faraz,
> Resubmitting with corrected format and attribution.
This version works, thanks!
> The following patch adds support for the CRC Application Specific
> Extension for Release 6 of the MIPS Architecture.
>
> The new crc* instructions are described in Section 3.2 of the MIPS
> Instruction Set v6.06
> (https://www.mips.com/?do-download=the-mips32-instruction-set-v6-06)
> and the MIPS64 Instruction Set v6.06
> (https://www.mips.com/?do-download=the-mips64-instruction-set-v6-06)
Is the above the intended commit description? If so, then based on
previous experience I think quoting web links is not going to work as
they change every so often. Quoting actual document titles/revisions
might be better as they can be used for a search query.
If not, then please use the GIT format, observing that everything above
a leading `---' mark goes into the description (and will be reviewed)
and what follows is any further discussion meant not to be recorded in
the repository; e.g. this is a good place to mention how the change has
been verified.
Speaking of which this change causes test suite failures for several MIPS
targets:
mips-sgi-irix6 +FAIL: MIPS CRC (mips32r6)
mips64-freebsd +FAIL: MIPS CRC (mips32r6)
mips64-img-linux +FAIL: MIPS CRC (mips32r6)
mips64-linux +FAIL: MIPS CRC (mips32r6)
mips64-mti-linux +FAIL: MIPS CRC (mips32r6)
mips64-openbsd +FAIL: MIPS CRC (mips32r6)
mips64el-freebsd +FAIL: MIPS CRC (mips32r6)
mips64el-img-linux +FAIL: MIPS CRC (mips32r6)
mips64el-linux +FAIL: MIPS CRC (mips32r6)
mips64el-mti-linux +FAIL: MIPS CRC (mips32r6)
mips64el-openbsd +FAIL: MIPS CRC (mips32r6)
-- can you please look into it?
> opcodes/
> * mips-dis.c (mips_arch_choices): Add ASE_CRC to mips32r6 and
> mips64r6.
> (mips_arch_choices): Add ASE_CRC64 to mips64r6.
Please don't repeat the name of the entity modified.
> * mips-opc.c (CRC and CRC64): New macros.
Use a comma to separate entity names, i.e. (CRC, CRC64).
> (mips_builtin_opcodes): Define crc32b, crc32h, crc32w,
> crc32cb, crc32ch and crc32cw for CRC. Define crc32d and
> crc32cd for CRC64.
Two spaces after a full stop please.
> diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
> index 040a373..84e447f 100644
> --- a/gas/config/tc-mips.c
> +++ b/gas/config/tc-mips.c
> @@ -1526,6 +1526,8 @@ enum options
> OPTION_NAN,
> OPTION_ODD_SPREG,
> OPTION_NO_ODD_SPREG,
> + OPTION_CRC,
> + OPTION_NO_CRC,
Please group these with the other ASE options, i.e. place immediately
following OPTION_NO_MIPS16E2 and matching the order in `md_longopts'.
> diff --git a/gas/doc/as.texinfo b/gas/doc/as.texinfo
> index d37a1d6..9d025cd 100644
> --- a/gas/doc/as.texinfo
> +++ b/gas/doc/as.texinfo
> @@ -1531,6 +1532,12 @@ Generate code for the MCU Application Specific Extension.
> This tells the assembler to accept MCU instructions.
> @samp{-mno-mcu} turns off this option.
>
> +@item -mcrc
> +@itemx -mno-crc
> +Generate code for the MIPS cyclic redundancy check (CRC) Application
> +Specific Extension. This tells the assembler to accept CRC instructions.
Two spaces after a full stop please.
> diff --git a/gas/doc/c-mips.texi b/gas/doc/c-mips.texi
> index a430b0d..8cd78b7 100644
> --- a/gas/doc/c-mips.texi
> +++ b/gas/doc/c-mips.texi
> @@ -234,6 +234,13 @@ Generate code for the Virtualization Application Specific Extension.
> This tells the assembler to accept Virtualization instructions.
> @samp{-mno-virt} turns off this option.
>
> +@item -mcrc
> +@itemx -mno-crc
> +Generate code for the cyclic redundancy check (CRC) Application Specific
> +Extension.
> +This tells the assembler to accept CRC instructions.
Please merge the last sentence with the previous line.
> diff --git a/gas/testsuite/gas/mips/ase-errors-1.l b/gas/testsuite/gas/mips/ase-errors-1.l
> index f989982..5037990 100644
> --- a/gas/testsuite/gas/mips/ase-errors-1.l
> +++ b/gas/testsuite/gas/mips/ase-errors-1.l
> @@ -40,3 +40,8 @@
> # ----------------------------------------------------------------------------
> .*:100: Warning: the `eva' extension requires MIPS32 revision 2 or greater
> .*:103: Error: opcode not supported.* `lbue \$4,16\(\$5\)'
> +# ----------------------------------------------------------------------------
> +.*:108: Error: invalid operands `crc32h \$4,\$7,\$5'
I think an operand validity check does not belong to an ASE
enable/disable test. Please move it to a separate case.
> diff --git a/gas/testsuite/gas/mips/ase-errors-1.s b/gas/testsuite/gas/mips/ase-errors-1.s
> index c5201c3..5cc07e4 100644
> --- a/gas/testsuite/gas/mips/ase-errors-1.s
> +++ b/gas/testsuite/gas/mips/ase-errors-1.s
> @@ -102,6 +102,16 @@
> .set noeva
> lbue $4,16($5) # ERROR: eva not enabled
>
> + .set mips32r6
> + .set crc # OK
> + crc32b $4,$7,$4 # OK
> + crc32h $4,$7,$5 # ERROR: Invalid operands
> + crc32d $4,$7,$4 # ERROR: 64-bit only
> + .set mips32r5 # ERROR: too low
> + crc32b $4,$7,$4 # OK
> + .set nocrc
> + crc32b $4,$7,$4 # ERROR: crc not enabled
This source file uses a space to separate mnemonics from operands, so
please adjust accordingly.
> diff --git a/gas/testsuite/gas/mips/ase-errors-2.l b/gas/testsuite/gas/mips/ase-errors-2.l
> index 4c24690..88fd0af 100644
> --- a/gas/testsuite/gas/mips/ase-errors-2.l
> +++ b/gas/testsuite/gas/mips/ase-errors-2.l
> @@ -32,3 +32,8 @@
> # ----------------------------------------------------------------------------
> .*:84: Warning: the `eva' extension requires MIPS64 revision 2 or greater
> .*:87: Error: opcode not supported.* `lbue \$4,16\(\$5\)'
> +# ----------------------------------------------------------------------------
> +.*:93: Error: invalid operands `crc32h \$4,\$7,\$5'
Again, please move it to a separate case.
> diff --git a/gas/testsuite/gas/mips/ase-errors-2.s b/gas/testsuite/gas/mips/ase-errors-2.s
> index 4a17e4f..fcb9fa7 100644
> --- a/gas/testsuite/gas/mips/ase-errors-2.s
> +++ b/gas/testsuite/gas/mips/ase-errors-2.s
> @@ -86,6 +86,18 @@
> .set noeva
> lbue $4,16($5) # ERROR: eva not enabled
>
> + .set mips64r6
> + .set crc # OK
> + crc32b $4,$7,$4 # OK
> + crc32d $4,$7,$4 # OK
> + crc32h $4,$7,$5 # ERROR: Invalid operands
> + .set mips64r5 # ERROR: too low
> + crc32b $4,$7,$4 # OK
> + crc32d $4,$7,$4 # OK
> + .set nocrc
> + crc32b $4,$7,$4 # ERROR: crc not enabled
> + crc32d $4,$7,$4 # ERROR: crc not enabled
Please use a space rather than a tab too.
> diff --git a/gas/testsuite/gas/mips/crc.s b/gas/testsuite/gas/mips/crc.s
> new file mode 100644
> index 0000000..3a8fb64
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/crc.s
> @@ -0,0 +1,15 @@
> + .set noreorder
> + .set noat
Hmm, any particular reason for these directives?
> diff --git a/gas/testsuite/gas/mips/crc64.s b/gas/testsuite/gas/mips/crc64.s
> new file mode 100644
> index 0000000..412c363
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/crc64.s
> @@ -0,0 +1,11 @@
> + .set noreorder
> + .set noat
And here?
> diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
> index 94c4506..1dd460d 100644
> --- a/gas/testsuite/gas/mips/mips.exp
> +++ b/gas/testsuite/gas/mips/mips.exp
> @@ -2050,4 +2050,7 @@ if { [istarget mips*-*-vxworks*] } {
>
> run_list_test_arches "r6-branch-constraints" "-32" \
> [mips_arch_list_matching mips32r6]
> +
> + run_dump_test_arches "crc" [mips_arch_list_matching mips32r6]
> + run_dump_test_arches "crc64" [mips_arch_list_matching mips64r6]
Please align [mips_arch_list_matching mips32r6] with tabs to column #40.
> diff --git a/include/opcode/mips.h b/include/opcode/mips.h
> index ceae9ec..93e1ef6 100644
> --- a/include/opcode/mips.h
> +++ b/include/opcode/mips.h
> @@ -1294,6 +1294,9 @@ static const unsigned int mips_isa_table[] = {
> /* The Virtualization ASE has eXtended Physical Addressing (XPA)
> instructions which are only valid when both ASEs are enabled. */
> #define ASE_XPA_VIRT 0x00020000
> +/* Cyclic redundancy check (CRC) ASE */
Full stop followed by two spaces please.
> +#define ASE_CRC 0x00100000
> +#define ASE_CRC64 0x00200000
Please allocate bits sequentially here.
> diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c
> index 19fca40..9bb49a4 100644
> --- a/opcodes/mips-opc.c
> +++ b/opcodes/mips-opc.c
> @@ -3160,6 +3164,15 @@ const struct mips_opcode mips_builtin_opcodes[] =
> {"move.v", "+d,+e", 0x78be0019, 0xffff003f, WR_1|RD_2, 0, 0, MSA, 0 },
> {"lsa", "d,v,t,+~", 0x00000005, 0xfc00073f, WR_1|RD_2|RD_3, 0, I37, MSA, 0 },
> {"dlsa", "d,v,t,+~", 0x00000015, 0xfc00073f, WR_1|RD_2|RD_3, 0, I69, MSA64, 0 },
> +/* MIPS cyclic redundancy check (CRC) ASE. */
> +{"crc32b", "t,s,-d", 0x7c00000f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32h", "t,s,-d", 0x7c00004f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32w", "t,s,-d", 0x7c00008f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32d", "t,s,-d", 0x7c0000cf, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC64, 0 },
> +{"crc32cb", "t,s,-d", 0x7c00010f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32ch", "t,s,-d", 0x7c00014f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32cw", "t,s,-d", 0x7c00018f, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC, 0 },
> +{"crc32cd", "t,s,-d", 0x7c0001cf, 0xfc00ffff, MOD_1|RD_2, 0, 0, CRC64, 0 },
Please move these below the R6 instruction block later on in this table,
and add an empty line above the comment.
Please also add a NEWS entry.
Maciej
More information about the Binutils
mailing list