[AArch64,4/4] Define TC_ADDRESS_BYTES for GAS

Message ID 06d11737-ef5a-05e7-24d1-8541bfc94e09@foss.arm.com
State New
Headers show

Commit Message

Jiong Wang Dec. 5, 2016, 5:53 p.m.
Address bytes should be 4 under ILP32, otherwise ".dc.a" will generate 64bit
relocation.

The default address_per_bytes is define in cpu-aarch64.c, current it's 8 for
both LP64 and ILP32, whether we should change it to 4 as well as word_per_bytes
looks to me is another issue we need to investigate.

gas/
2016-12-05  Jiong Wang<jiong.wang@arm.com>

         * config/tc-aarch64.h (TC_ADDRESS_BYTES): New define.
         (aarch64_address_bytes): New declaration.
         * config/tc-aarch64.c (aarch64_address_bytes): New function.

Comments

Jiong Wang Dec. 6, 2016, 11:09 a.m. | #1
On 06/12/16 10:50, Andreas Schwab wrote:
> On Dez 05 2016, Jiong Wang <jiong.wang@foss.arm.com> wrote:

>

>> The default address_per_bytes is define in cpu-aarch64.c, current it's 8 for

>> both LP64 and ILP32, whether we should change it to 4 as well as word_per_bytes

>> looks to me is another issue we need to investigate.

> This works for me and fixes quite a few failures:


Hi Andreas,
   Thanks very much for verifying this.

Yury, does Andreas's cpu-aarch64.c change works well with Cavium's ILP32 
environment, mind a full testing on it?

Thanks.

>

> 	* cpu-aarch64.c (N): Add argument ADDRSIZE.

> 	(bfd_aarch64_arch_ilp32): Set it to 32.

> 	(bfd_aarch64_arch): Set it to 64.

> ---

>   bfd/cpu-aarch64.c | 8 ++++----

>   1 file changed, 4 insertions(+), 4 deletions(-)

>

> diff --git a/bfd/cpu-aarch64.c b/bfd/cpu-aarch64.c

> index 596d24190b..af2da8455d 100644

> --- a/bfd/cpu-aarch64.c

> +++ b/bfd/cpu-aarch64.c

> @@ -100,16 +100,16 @@ scan (const struct bfd_arch_info *info, const char *string)

>     return FALSE;

>   }

>   

> -#define N(NUMBER, PRINT, DEFAULT, NEXT)				\

> -  { 64, 64, 8, bfd_arch_aarch64, NUMBER,			\

> +#define N(NUMBER, PRINT, ADDRSIZE, DEFAULT, NEXT)		\

> +  { 64, ADDRSIZE, 8, bfd_arch_aarch64, NUMBER,			\

>       "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

>       bfd_arch_default_fill, NEXT }

>   

>   static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =

> -  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);

> +  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 32, FALSE, NULL);

>   

>   const bfd_arch_info_type bfd_aarch64_arch =

> -  N (0, "aarch64", TRUE, &bfd_aarch64_arch_ilp32);

> +  N (0, "aarch64", 64, TRUE, &bfd_aarch64_arch_ilp32);

>   

>   bfd_boolean

>   bfd_is_aarch64_special_symbol_name (const char *name, int type)
Yury Norov Dec. 6, 2016, 11:11 a.m. | #2
On Tue, Dec 06, 2016 at 11:50:25AM +0100, Andreas Schwab wrote:
> On Dez 05 2016, Jiong Wang <jiong.wang@foss.arm.com> wrote:

> 

> > The default address_per_bytes is define in cpu-aarch64.c, current it's 8 for

> > both LP64 and ILP32, whether we should change it to 4 as well as word_per_bytes

> > looks to me is another issue we need to investigate.

> 

> This works for me and fixes quite a few failures:

> 

> 	* cpu-aarch64.c (N): Add argument ADDRSIZE.

> 	(bfd_aarch64_arch_ilp32): Set it to 32.

> 	(bfd_aarch64_arch): Set it to 64.

> ---

>  bfd/cpu-aarch64.c | 8 ++++----

>  1 file changed, 4 insertions(+), 4 deletions(-)

> 

> diff --git a/bfd/cpu-aarch64.c b/bfd/cpu-aarch64.c

> index 596d24190b..af2da8455d 100644

> --- a/bfd/cpu-aarch64.c

> +++ b/bfd/cpu-aarch64.c

> @@ -100,16 +100,16 @@ scan (const struct bfd_arch_info *info, const char *string)

>    return FALSE;

>  }

>  

> -#define N(NUMBER, PRINT, DEFAULT, NEXT)				\

> -  { 64, 64, 8, bfd_arch_aarch64, NUMBER,			\

> +#define N(NUMBER, PRINT, ADDRSIZE, DEFAULT, NEXT)		\

> +  { 64, ADDRSIZE, 8, bfd_arch_aarch64, NUMBER,			\

>      "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

>      bfd_arch_default_fill, NEXT }

>  

>  static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =

> -  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);

> +  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", 32, FALSE, NULL);

>  

>  const bfd_arch_info_type bfd_aarch64_arch =

> -  N (0, "aarch64", TRUE, &bfd_aarch64_arch_ilp32);

> +  N (0, "aarch64", 64, TRUE, &bfd_aarch64_arch_ilp32);


The first argument of macro is bits_per_word. I think it should also
be 32. Though, I didn't test it yet...

Yury.
Yury Norov Dec. 7, 2016, 4:10 p.m. | #3
> This is my version of the change. The same idea, but I also set

> bits_per_word to 32:

> 

> diff --git a/bfd/cpu-aarch64.c b/bfd/cpu-aarch64.c

> index 596d241..cc98e2e 100644

> --- a/bfd/cpu-aarch64.c

> +++ b/bfd/cpu-aarch64.c

> @@ -105,8 +105,13 @@ scan (const struct bfd_arch_info *info, const char *string)

>      "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

>      bfd_arch_default_fill, NEXT }

>  

> +#define N32(NUMBER, PRINT, DEFAULT, NEXT)				\

> +  { 32, 32, 8, bfd_arch_aarch64, NUMBER,			\

> +    "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

> +    bfd_arch_default_fill, NEXT }

> +

>  static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =

> -  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);

> +  N32 (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);

>  

>  const bfd_arch_info_type bfd_aarch64_arch =

>    N (0, "aarch64", TRUE, &bfd_aarch64_arch_ilp32);

> 

> ---

> 

> And this is before/after test results:

> 

> After                                                   Before

> 						>	FAIL: c++-types-check

> FAIL: elf/tst-tls-manydynamic			<

>                                                 >	FAIL: iconvdata/mtrace-tst-loading

> 						>	FAIL: iconvdata/tst-loading

> 						>	FAIL: iconvdata/tst-tables

>                                 		>	FAIL: localedata/mtrace-tst-leaks

> 						>	FAIL: localedata/tst-leaks

> 						>	FAIL: posix/tst-getaddrinfo4

> 						>	FAIL: posix/tst-getaddrinfo5

> 						>	FAIL: posix/tst-regex2

> 

> Pretty good to me. I'm not experienced in ld logic, but tests show

> that changing both wordsize and addrsize is right idea. If no

> objections here, I'll send the patch to binutils maillist.

> 

> Steve, could you try this patch with real hardware and report here?


Hi Andreas,

Are you OK with this change? If no objections, I'd like to send it,
and it would be great if you let me  add you as the co-author to
changelog. BTW, tst-tls-manydynamic is not a real regression. It
also fails for LP64 in my setup.

Yury.
Andreas Schwab Dec. 7, 2016, 4:17 p.m. | #4
On Dez 07 2016, Yury Norov <ynorov@caviumnetworks.com> wrote:

>> This is my version of the change. The same idea, but I also set

>> bits_per_word to 32:

>> 

>> diff --git a/bfd/cpu-aarch64.c b/bfd/cpu-aarch64.c

>> index 596d241..cc98e2e 100644

>> --- a/bfd/cpu-aarch64.c

>> +++ b/bfd/cpu-aarch64.c

>> @@ -105,8 +105,13 @@ scan (const struct bfd_arch_info *info, const char *string)

>>      "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

>>      bfd_arch_default_fill, NEXT }

>>  

>> +#define N32(NUMBER, PRINT, DEFAULT, NEXT)				\

>> +  { 32, 32, 8, bfd_arch_aarch64, NUMBER,			\

>> +    "aarch64", PRINT, 4, DEFAULT, compatible, scan,		\

>> +    bfd_arch_default_fill, NEXT }

>> +

>>  static const bfd_arch_info_type bfd_aarch64_arch_ilp32 =

>> -  N (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);

>> +  N32 (bfd_mach_aarch64_ilp32, "aarch64:ilp32", FALSE, NULL);


It doesn't make much sense to define another macro and use it only once.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Patch hide | download patch | download mbox

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 1d25422..af2b5ee 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -8925,3 +8925,14 @@  aarch64_copy_symbol_attributes (symbolS * dest, symbolS * src)
 {
   AARCH64_GET_FLAG (dest) = AARCH64_GET_FLAG (src);
 }
+
+/* Export the ABI address size for use by TC_ADDRESS_BYTES for the
+   purpose of the `.dc.a' internal pseudo-op.  */
+
+int
+aarch64_address_bytes (void)
+{
+  if ((stdoutput->arch_info->mach & bfd_mach_aarch64_ilp32))
+    return 4;
+  return stdoutput->arch_info->bits_per_address / 8;
+}
diff --git a/gas/config/tc-aarch64.h b/gas/config/tc-aarch64.h
index e1bc3a6..f2c5936 100644
--- a/gas/config/tc-aarch64.h
+++ b/gas/config/tc-aarch64.h
@@ -237,4 +237,7 @@  void tc_pe_dwarf2_emit_offset (symbolS *, unsigned int);
 
 #endif /* TE_PE */
 
+#define TC_ADDRESS_BYTES aarch64_address_bytes
+extern int aarch64_address_bytes (void);
+
 #endif /* TC_AARCH64 */