[PATCH, MIPS] Ensure default ASEs are applied for an architecture
Maciej W. Rozycki
macro@imgtec.com
Wed Mar 22 11:15:00 GMT 2017
Hi Matthew,
Thank you for your submission.
Your proposed change looks mostly good to me, except I think we don't
actually need a separate `init_ase' member; see below. See a couple of
minor nits too.
> gas/
>
Please remember to quote "PR gas/21219" along with the ChangeLog entry,
both in the commit description and the actual file, and preferably also in
the commit heading (e-mail subject).
> * config/tc-mips.c (struct mips_set_options): Add init_ase
> field.
> (file_mips_opts): Zero init_ase.
> (mips_opts): Likewise.
> (file_mips_check_options): Use file_mips_opts.init_ase to
> set enabled ases.
Please capitalise ASEs correctly.
> @@ -3918,8 +3926,6 @@ mips_check_options (struct mips_set_options *opts, bfd_boolean abi_checks)
> static void
> file_mips_check_options (void)
> {
> - const struct mips_cpu_info *arch_info = 0;
> -
> if (file_mips_opts_checked)
> return;
>
> @@ -3962,8 +3968,6 @@ file_mips_check_options (void)
> file_mips_opts.fp = 32;
> }
>
> - arch_info = mips_cpu_info_from_arch (file_mips_opts.arch);
> -
> /* Disable operations on odd-numbered floating-point registers by default
> when using the FPXX ABI. */
> if (file_mips_opts.oddspreg < 0)
> @@ -4007,7 +4011,7 @@ file_mips_check_options (void)
>
> /* If the user didn't explicitly select or deselect a particular ASE,
> use the default setting for the CPU. */
> - file_mips_opts.ase |= (arch_info->ase & ~file_ase_explicit);
> + file_mips_opts.ase |= (file_mips_opts.init_ase & ~file_ase_explicit);
This is the first place where `file_mips_opts.ase' is ever set AFAICT, so
I think all that you need here is:
file_mips_opts.ase &= ~file_ase_explicit;
(and `arch_info' can of course go, as you did), with the hunk below
changed accordingly:
> @@ -14769,6 +14773,7 @@ mips_after_parse_args (void)
>
> file_mips_opts.arch = arch_info->cpu;
> file_mips_opts.isa = arch_info->isa;
> + file_mips_opts.init_ase = arch_info->ase;
file_mips_opts.ase = arch_info->ase;
The rest of the change can then go. Please verify that this does what's
intended.
As a follow-up update I think `.set arch=' should set `mips_opts.ase'
accordingly too (possibly with `file_ase_explicit' applied), which would
be consistent and should resolve some issues we have seen with the Linux
kernel.
Also as another followup I think we should record `arch_info->name' too
for reporting, so that rather than say:
Error: opcode not supported on this processor: mips32r2 (mips32r2)
a more meaningful:
Error: opcode not supported on this processor: 34kc (mips32r2)
message is produced, reporting the actual architecture chosen even for
those that do not have a dedicated BFD architecture. Then
`mips_cpu_info_from_arch' can go altogether.
I can handle the two followups myself unless you're keen to.
For further nits see below.
> diff --git a/gas/testsuite/gas/mips/34kc-mt.d b/gas/testsuite/gas/mips/34kc-mt.d
> new file mode 100644
> index 0000000..f5ac635
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/34kc-mt.d
> @@ -0,0 +1,12 @@
> +#objdump: -dr --prefix-addresses -M reg-names=numeric
> +#name: MIPS 34kc MT ASE default
Please capitalise 34Kc correctly.
> +#as: -32 -march=34kc
> +
> +# Test the abs macro.
Pasto here?
> +
> +.*: +file format .*mips.*
> +
> +Disassembly of section .text:
> +0+0000 <[^>]*> mttc0 \$4,\$25,1
> +0+0004 <[^>]*> mftc0 \$25,\$4
> + ...
Please escape `.' characters to be matched literally here and with
`.text'. We do have special handling in the test framework for implicit
escaping `.' in `.text', `.data' and `.bss' in dump patterns, but not for
other section names and certainly not for other cases, so please keep our
test cases consistent.
> diff --git a/gas/testsuite/gas/mips/34kc-mt.s b/gas/testsuite/gas/mips/34kc-mt.s
> new file mode 100644
> index 0000000..ab68fdb
> --- /dev/null
> +++ b/gas/testsuite/gas/mips/34kc-mt.s
> @@ -0,0 +1,5 @@
> +# Source file used to test that the MT ase is enabled by default for 34kc.
34Kc again, and also ASE.
Please rework and resubmit.
Maciej
More information about the Binutils
mailing list