Re: [PATCH 1/8] MIPS: Replace assembly isa level directives with macros

Jiaxun Yang
2023-04-20T19:29:16Z

> 2023年4月20日 17:43，Nick Desaulniers <ndesaulniers@google.com> 写道：
>
> On Sun, Apr 9, 2023 at 3:43 AM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>> There are tons of .set assembly isa level directives in tree
>> to set current isa level to a higher level to enable some features.
>>
>> However as nowadays MIPS is shuffling around opcodes, a single ISA
>
> What do you mean by "shuffling around opcodes?"

Newer MIPS ISA (i.e. MIPS Release 6) had changed opcodes for some instructions.
Mostly moving around opcode fields.

>
>> level setting may result wrong opcode in binary. We had MIPS_ISA_LEVEL
>> macros to determine most appropriate superset of ISA level, just do a
>> tree wide replacement to take advantage of those macros.
>
> Hi Jiaxun,
> Thanks for the patch! Do you have more info about how this change
> relates to LLVM? Is clang having a hard time assembling some of these
> files with the given `.set arch=` directives without this patch?
> Seems perhaps related to just {mips32r5, mips64r5} perhaps?

Yes, GAS and LLVM sometimes have different opinions on what a instruction
feature should belong to. Personally I think there is no right or wrong in most case.

So generally when we try to use some inline assembly features that toolchain
may consider belongs to higher ISA level we will use `.set mips64r2` directives.

Having this patch just unified the defined arch across the tree, so it happens to fix
some cases where `.set` was given a improper option.

>
> More comments below, inline.
>>
>> -#ifndef CONFIG_AS_HAS_VIRT
>> +#if MIPS_ISA_REV < 5
>> +/* VZ ASE should be able to work with Release 2 but LLVM thinks it needs R5 */
>> +#define MIPS_ISA_LEVEL_VIRT "mips64r5"
>> +#else
>> +#define MIPS_ISA_LEVEL_VIRT MIPS_ISA_LEVEL
>> +#endif
>
> Do we have a bug on file for LLVM for this? If not, please file one.
> Either way, please include a link in the commit message (and maybe in
> a comment in the sources as well).

This is a perfect example, strictly speaking VZ extension is a addition of MIPS Release 5,
LLVM is doing nothing wrong.
In reality we are happy to run MIPS Release 2 Kernel on Release 5 CPUs, so GAS decided
to allow it live with any MIPS level.

Perhaps I should reword the comment.

Thanks.
- Jiaxun