RE: [PATCH v3 02/44] bfd: add intelgt target to BFD

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Hello Jan,

Thanks for your review.

>> @@ -198,6 +198,7 @@ fido*)		 targ_archs=bfd_m68k_arch ;;
>>  hppa*)		 targ_archs=bfd_hppa_arch ;;
>>  i[3-7]86)	 targ_archs=bfd_i386_arch ;;
>>  ia16)		 targ_archs=bfd_i386_arch ;;
>> +intelgt)	 targ_archs=bfd_intelgt_arch ;;
>>  kvx)		 targ_archs=bfd_kvx_arch ;;
>>  loongarch*)	 targ_archs=bfd_loongarch_arch ;;
>>  m6811*|m68hc11*) targ_archs="bfd_m68hc11_arch bfd_m68hc12_arch
>bfd_m9s12x_arch bfd_m9s12xg_arch" ;;
>> @@ -705,12 +706,12 @@ case "${targ}" in
>>      ;;
>>    x86_64-*-linux-*)
>>      targ_defvec=x86_64_elf64_vec
>> -    targ_selvecs="i386_elf32_vec iamcu_elf32_vec x86_64_elf32_vec
>i386_pei_vec x86_64_pe_vec x86_64_pei_vec"
>> +    targ_selvecs="i386_elf32_vec iamcu_elf32_vec x86_64_elf32_vec
>i386_pei_vec x86_64_pe_vec x86_64_pei_vec intelgt_elf64_vec"
>>      want64=true
>>      ;;
>>    x86_64-*-mingw* | x86_64-*-pe | x86_64-*-pep | x86_64-*-cygwin)
>>      targ_defvec=x86_64_pe_vec
>> -    targ_selvecs="x86_64_pe_vec x86_64_pei_vec x86_64_pe_big_vec
>x86_64_elf64_vec i386_pe_vec i386_pei_vec i386_elf32_vec iamcu_elf32_vec
>pdb_vec"
>> +    targ_selvecs="x86_64_pe_vec x86_64_pei_vec x86_64_pe_big_vec
>x86_64_elf64_vec i386_pe_vec i386_pei_vec i386_elf32_vec iamcu_elf32_vec
>pdb_vec intelgt_elf64_vec intelgt_legacy_elf64_vec"
>>      want64=true
>>      targ_underscore=no
>>      ;;
>
>I'm not convinced of forcing this as secondary target onto (about) everyone.

Indeed, this doesn't seem necessary.  We'll remove that part.

>> +#ifdef BFD64
>> +  intelgt-*-elf)
>> +    targ_defvec=intelgt_elf64_vec
>> +    targ_selvecs="intelgt_elf64_vec"
>
>Why would targ_selvecs need to hold what targ_defvec already has?

We'll remove that, too.

>> +  HOWTO (R_ZE_SYM_ADDR,		/* type */
>> +	 0,			/* rightshift */
>> +	 2,			/* size (0 = byte, 1 = short, 2 = long) */
>> +	 64,			/* bitsize */
>> +	 false,			/* pc_relative */
>> +	 0,			/* bitpos */
>> +	 complain_overflow_unsigned,	/* complain_on_overflow */
>> +	 bfd_elf_generic_reloc, /* special_function */
>> +	 "R_ZE_SYM_ADDR",	/* name */
>> +	 false,			/* partial_inplace */
>> +	 MINUS_ONE,		/* src_mask */
>
>Based on the arch apparently using RELA relocations, src_mask shouldn't be
>non-zero anywhere aiui.

We don't really need all this, I believe.  We will drop the reloc map and howto
table  in v4 and just keep the reloc numbers.

>>        if (rtype == NULL)
>> @@ -15589,6 +15594,8 @@ is_32bit_abs_reloc (Filedata * filedata, unsigned
>int reloc_type)
>>        return reloc_type == 1; /* R_XTENSA_32.  */
>>      case EM_Z80:
>>        return reloc_type == 6; /* R_Z80_32.  */
>> +    case EM_INTELGT:
>> +      return reloc_type == 2; /* R_ZE_SYM_ADDR_32 */
>
>Not sure here as well, when this can be the lower half of a 64-bit address.

We'll drop that, too.

>> +START_RELOC_NUMBERS (elf_intelgt_reloc_type)
>> +  RELOC_NUMBER (R_ZE_NONE,			     0)
>> +  /* 64-bit address.  */
>> +  RELOC_NUMBER (R_ZE_SYM_ADDR,			     1)
>> +  /* 32-bit address or lower 32-bit of a 64-bit address.  */
>> +  RELOC_NUMBER (R_ZE_SYM_ADDR_32,		     2)
>> +  /* Higher 32bits of a 64-bit address.  */
>> +  RELOC_NUMBER (R_ZE_SYM_ADDR32_HI,		     3)
>> +  /* 32-bit field of payload offset of per-thread data.  */
>> +  RELOC_NUMBER (R_PER_THREAD_PAYLOAD_OFFSET_32,      4)
>
>Why is this not R_ZE_PER_THREAD_PAYLOAD_OFFSET_32?

That's a typo.  I'm adding a few to v4, all with ZE prefix.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928