Re: [PATCH v4] MIPS: Sync elf.h from binutils

["Maciej W. Rozycki" <macro@orcam.me.uk>]

On Fri, 2 Jun 2023, Ying Huang wrote:

> From: Ying Huang <ying.huang@oss.cipunited.com>
> 
> Add new definitions for the MIPS target, specifically: relocation
> types, machine flags, section type names, and object attribute tags
> and values. On MIPS64, up to three relocations may be specified

 Two spaces after a full stop please, as required by the GNU Coding 
Standards.

> diff --git a/elf/elf.h b/elf/elf.h
> index ac7032b7a5..71c18099ec 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -1685,11 +1688,18 @@ typedef struct
>  #define EF_MIPS_PIC		2     /* Contains PIC code.  */
>  #define EF_MIPS_CPIC		4     /* Uses PIC calling sequence.  */
>  #define EF_MIPS_XGOT		8
> -#define EF_MIPS_64BIT_WHIRL	16
> +#define EF_MIPS_UCODE		16
>  #define EF_MIPS_ABI2		32
>  #define EF_MIPS_ABI_ON32	64
> +#define EF_MIPS_OPTIONS_FIRST	0x00000080 /* Process the .MIPS.options section first by ld */
> +#define EF_MIPS_32BITMODE	0x00000100 /* Indicates code compiled for a 64-bit machine in
> +					      32-bit mode(regs are 32-bits wide).  */

 Please wrap the comments such as to stay within 79 columns.  Also a space 
is missing before the opening paren above.

>  #define EF_MIPS_FP64		512  /* Uses FP64 (12 callee-saved).  */
>  #define EF_MIPS_NAN2008	1024  /* Uses IEEE 754-2008 NaN encoding.  */
> +#define EF_MIPS_ARCH_ASE	0x0f000000 /* Architectural Extensions used by this file */

 Likewise wrap the comment.

> +#define EF_MIPS_ARCH_ASE_MDMX	0x08000000 /* Use MDMX multimedia extensions */
> +#define EF_MIPS_ARCH_ASE_M16	0x04000000 /* Use MIPS-16 ISA extensions */
> +#define EF_MIPS_ARCH_ASE_MICROMIPS	0x02000000 /* Use MICROMIPS ISA extensions.  */

 This definition overruns the column alignment; I think the most readable 
conforming formatting will be:

#define EF_MIPS_ARCH_ASE_MICROMIPS \
				0x02000000 /* Use MICROMIPS ISA extensions.  */

> @@ -1703,6 +1713,35 @@ typedef struct
>  #define EF_MIPS_ARCH_64		0x60000000 /* MIPS64 code.  */
>  #define EF_MIPS_ARCH_32R2	0x70000000 /* MIPS32r2 code.  */
>  #define EF_MIPS_ARCH_64R2	0x80000000 /* MIPS64r2 code.  */
> +#define E_MIPS_ARCH_32R6	0x90000000 /* -mips32r6 code.  */
> +#define E_MIPS_ARCH_64R6	0xa0000000 /* -mips64r6 code.  */

 I think all new additions are supposed to start with EF_ rather than E_.  
IIUC the existence of E_ macros has something to do with the old SVR4 ABI 
registry maintained by SCO long ago: you could only add new EF_ macros 
once they've been registered with SCO (someone please correct me if I'm 
wrong).

 Also I think the new comments will best be spelt MIPS32r6/MIPS64r6 rather 
than -mips32r6/-mips64r6 for consistency with MIPS32r2/MIPS64r2 above.

> +#define EF_MIPS_ABI		0x0000F000 /* The ABI of the file.  Also see EF_MIPS_ABI2 above. */
> +#define E_MIPS_ABI_O32		0x00001000 /* The original o32 abi. */
> +#define E_MIPS_ABI_O64		0x00002000 /* O32 extended to work on 64 bit architectures */

 These comments need to be wrapped too.  Also each comment needs to end 
with a full stop followed by two spaces.

> +#define E_MIPS_ABI_EABI32	0x00003000 /* EABI in 32 bit mode */
> +#define E_MIPS_ABI_EABI64	0x00004000 /* EABI in 64 bit mode */

 Missing full stops followed by two spaces here as well.

> +#define E_MIPS_MACH_SB1        0x008a0000

 This line still uses spaces rather than tabs.

> +#define E_MIPS_MACH_XLR     	0x008c0000

 Likewise.

> +#define R_MIPS_PC21_S2          60
> +#define R_MIPS_PC26_S2          61
> +#define R_MIPS_PC18_S3          62
> +#define R_MIPS_PC19_S2          63
> +#define R_MIPS_PCHI16           64
> +#define R_MIPS_PCLO16           65
> +#define R_MIPS16_26             100
> +#define R_MIPS16_GPREL          101
> +#define R_MIPS16_GOT16          102
> +#define R_MIPS16_CALL16         103
> +#define R_MIPS16_HI16           104
> +#define R_MIPS16_LO16           105
> +#define R_MIPS16_TLS_GD         106
> +#define R_MIPS16_TLS_LDM        107
> +#define R_MIPS16_TLS_DTPREL_HI16 108
> +#define R_MIPS16_TLS_DTPREL_LO16 109
> +#define R_MIPS16_TLS_GOTTPREL   110
> +#define R_MIPS16_TLS_TPREL_HI16 111
> +#define R_MIPS16_TLS_TPREL_LO16 112
> +#define R_MIPS16_PC16_S1        113

 Likewise all the lines above.

> +#define R_MIPS_RELATIVE         128
> +#define R_MICROMIPS_26_S1       133
> +#define R_MICROMIPS_HI16        134
> +#define R_MICROMIPS_LO16        135
> +#define R_MICROMIPS_GPREL16     136
> +#define R_MICROMIPS_LITERAL     137
> +#define R_MICROMIPS_GOT16       138
> +#define R_MICROMIPS_PC7_S1      139
> +#define R_MICROMIPS_PC10_S1     140
> +#define R_MICROMIPS_PC16_S1     141
> +#define R_MICROMIPS_CALL16      142
> +#define R_MICROMIPS_GOT_DISP    145
> +#define R_MICROMIPS_GOT_PAGE    146
> +#define R_MICROMIPS_GOT_OFST    147
> +#define R_MICROMIPS_GOT_HI16    148
> +#define R_MICROMIPS_GOT_LO16    149
> +#define R_MICROMIPS_SUB         150
> +#define R_MICROMIPS_HIGHER      151
> +#define R_MICROMIPS_HIGHEST     152
> +#define R_MICROMIPS_CALL_HI16   153
> +#define R_MICROMIPS_CALL_LO16   154
> +#define R_MICROMIPS_SCN_DISP    155
> +#define R_MICROMIPS_JALR        156
> +#define R_MICROMIPS_HI0_LO16    157
> +#define R_MICROMIPS_TLS_GD      162
> +#define R_MICROMIPS_TLS_LDM     163
> +#define R_MICROMIPS_TLS_DTPREL_HI16  164
> +#define R_MICROMIPS_TLS_DTPREL_LO16  165
> +#define R_MICROMIPS_TLS_GOTTPREL     166
> +#define R_MICROMIPS_TLS_TPREL_HI16   169
> +#define R_MICROMIPS_TLS_TPREL_LO16   170
> +#define R_MICROMIPS_GPREL7_S2   172
> +#define R_MICROMIPS_PC23_S2     173
> +#define R_MIPS_PC32             248
> +#define R_MIPS_EH               249
> +#define R_MIPS_GNU_REL16_S2     250
> +#define R_MIPS_GNU_VTINHERIT    253
> +#define R_MIPS_GNU_VTENTRY      254

 And these.  I realise this file has preexisting issues, but that does not 
mean new ones are supposed to be introduced.

  Maciej