This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][mips] Introduce '.set noase' to the mips assembler.


ping.

I have corrected the gas/ChangeLog below.
Adding mips maintainers Maciej W. Rozycki and Chenghua Xu.

Regards,
Egeyar


On 9/26/19 7:51 PM, Egeyar Bagcioglu wrote:
Hello,

I would like to propose the attached tiny patch, together with the lengthy explanation of its use case. Please review.

Currently, the mips assembler keeps, as a part of its runtime state, information regarding the accepted ISA and ASEs (Application Specific Extensions). It is possible to change the ISA or allowed extensions on the fly. It is also possible to use ".set push" to save the state regarding the accepted instructions, change them, and pop the previously pushed state afterwards.

Linux kernel takes advantage of this situation in its mips inline assembly functions. The assembler is asked to push into the stack the information of the target platform. The ISA is then set to mips3, which is the strictest, to make sure that the assembler generates an error if the inline assembler is not accepted by all mips platforms. At the end of the inline assembly, the previous state of the assembler is restored with a ".set pop":

.set push
.set mips3      #the base ISA
<assembly>
.set pop

The above usage works mostly, when no ASEs are enabled or when the enabled ASEs are accepted in mips3 due other settings (such as micromips being enabled, etc.). If, however, a conflicting ASE is enabled, the switch to mips3 causes the assembler to generate a warning per file because mips3 does not support that given ASE. While building a big project such as the kernel, one warning per file  with inline assembly means several hundreds of warnings in total.

I have considered different ways to solve this issue. If we implicitly disable the unsupported ASEs upon switching to a new ISA, we lose backwards compatibility. Any project which is -for whatever reason- generating some extension instructions that are not supported by the given ISA would start getting errors from the assembler. If we decide to turn off the warnings under whatever condition, the assembler would start accepting unsupported extension instructions without even a warning. This would, for example, undermine the kernel's use-case. A more-user-friendly option could be delaying the warning. We would not warn when a conflicting ISA is set. Instead, we would store this information, check the following instructions and only warn if/when we see the first instruction using a conflicting ASE. However, I believe the overhead of this approach could not be justified by the need.

The change I propose is to add a new ".set noase" directive to disable all the ASEs. This pseudo-op would be beneficial primarily within .set push/pop pairs of inline assembly functions as used by the kernel, by explicitly disabling all 19 extensions of mips architecture and avoiding the related warnings. An example usage of it would transform the inline assembly functions into the following format:

.set push
.set noase
.set mips3    #the base ISA
<assembly>
.set pop


As .set push/pop pairs also saves and restores the state of enabled ASEs, this would indeed serve the purpose of limiting the generation of extension instructions within that pair, without permanently changing which instructions are allowed. Having said that, the execution of this directive does not depend on the .set push/pop pairs. Therefore, it can also be used separately.

I do not know how likely it is for the kernel or other projects to use this new directive as it would depend on a very new binutils patch. Although, it might be applied together with a macro controlling its usage. Either way, I still see value in assembler supporting such a usage.

As it is new, this directive does not interfere with the existing regression tests. Therefore, you'll see a new test case in the patch as well.

The suggested gas/ChangeLog is
2019-09-26  Egeyar Bagcioglu  <egeyar.bagcioglu@oracle.com>

        * config/tc-mips.c (parse_code_option): Introduce ".set/.module
        noase" pseudo-op to disable all Application Specific Extensions.
        * testsuite/gas/mips/ase-errors-5.l: New test.
        * testsuite/gas/mips/ase-errors-5.s: New test source.
        * testsuite/gas/mips/mips.exp: Run the new test.


Please review and let me know what you think.

Thanks,
Egeyar


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]