| Message ID | 06d11737-ef5a-05e7-24d1-8541bfc94e09@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
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)
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.
> 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.
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."
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 */
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.