Message ID | 1545167083-16764-1-git-send-email-vgupta@synopsys.com |
---|---|
Headers | show |
Series | glibc port to ARC processors | expand |
On Tue, 18 Dec 2018, Vineet Gupta wrote: > | FAIL: elf/check-localplt # passes for build-many-glibcs.py > # buildroot builds with slightly different toggles (-Os) > # such that gcc generates an extra memcpy PLT call I fixed lots of such issues with -Os on various architectures; it should be possible to fix this one as well. (I haven't run build-many-glibcs.py with -Os lately, so it's possible more problems have crept in or appear with current compilers. When I was testing it, it failed for m68k - the m68k libm functions don't build with -Os because bits/mathinline.h doesn't get included then, which needs addressing anyway in some way to eliminate bits/mathinline.h from glibc - but worked for most or all other configurations.) > (d) We currently only support soft float ABI, although I'm working on hard float > and hopefully will have it ready sometime during the review of this series Could you confirm what the ABI entry or entries to go at <https://sourceware.org/glibc/wiki/ABIList> would look like? Note that we generally expect new ports to use different dynamic linker names for each ABI. This means separate names for hard float and soft float. Since it appears you support both big and little endian, it also means separate dynamic linker names for each of those (and corresponding GCC changes will be needed to pass the right dynamic linker names to ld's -dynamic-linker option). Then, each supported ABI should have a build in build-many-glibcs.py, and any unsupported ABI variants need to have a configure-time error (to avoid accidentally building a glibc incompatible with the correct ABI for such a port added in future).
On 12/18/18 2:52 PM, Joseph Myers wrote: > On Tue, 18 Dec 2018, Vineet Gupta wrote: > >> | FAIL: elf/check-localplt # passes for build-many-glibcs.py >> # buildroot builds with slightly different toggles (-Os) >> # such that gcc generates an extra memcpy PLT call > > I fixed lots of such issues with -Os on various architectures; it should > be possible to fix this one as well. (I haven't run build-many-glibcs.py > with -Os lately, so it's possible more problems have crept in or appear > with current compilers. When I was testing it, it failed for m68k - the > m68k libm functions don't build with -Os because bits/mathinline.h doesn't > get included then, which needs addressing anyway in some way to eliminate > bits/mathinline.h from glibc - but worked for most or all other > configurations.) The actual xcheck runs posted later on have been with buildroot built toolchain which defaults to -Os, so it works for ARC, except for this additional entry. At any rate, the generated code will likely be different for -Os and otherwise, does that mean entries get added conditionally to localplt.data etc ? >> (d) We currently only support soft float ABI, although I'm working on hard float >> and hopefully will have it ready sometime during the review of this series > > Could you confirm what the ABI entry or entries to go at > <https://sourceware.org/glibc/wiki/ABIList> would look like? soft-float LE: ld-linux-arc.so.2 hard-float LE: ld-linux-archf.so.2 (although this is just a preference) > Note that we generally expect new ports to use different dynamic linker > names for each ABI. This means separate names for hard float and soft > float. Since it appears you support both big and little endian, it also > means separate dynamic linker names for each of those (and corresponding > GCC changes will be needed to pass the right dynamic linker names to ld's > -dynamic-linker option). ARC historically has supported both LE/BE, but we are kind of moving away from BE and as far as I know, there's no-one actively working with BE (except 1 specific customer with legact cores). It is safe to assume we won't support BE and this is sort of easier said for ARC processors as they have many many hardware config knobs and not all are supported for Linux usecase, so BE can be considered as such. > Then, each supported ABI should have a build in > build-many-glibcs.py, and any unsupported ABI variants need to have a > configure-time error (to avoid accidentally building a glibc incompatible > with the correct ABI for such a port added in future). Right, at this point I only have soft-float and work is ongoing for hard. Thx for taking a look. -Vineet
Another general point: when posting a new port, could you include pointers to architecture and ABI reference manuals in case those are of relevance to the review? (URLs going directly to PDFs of those manuals are preferred.)
On Tue, 18 Dec 2018, Vineet Gupta wrote: > At any rate, the generated code will likely be different for -Os and otherwise, > does that mean entries get added conditionally to localplt.data etc ? Entries *can* have a "?" to indicate they are optional (although avoiding the PLT reference is better - see how include/string.h does asm redirection of mempcpy and stpcpy, or likewise in sysdeps/wordsize-32/divdi3-symbol-hacks.h, for example). > ARC historically has supported both LE/BE, but we are kind of moving > away from BE and as far as I know, there's no-one actively working with > BE (except 1 specific customer with legact cores). It is safe to assume > we won't support BE and this is sort of easier said for ARC processors > as they have many many hardware config knobs and not all are supported > for Linux usecase, so BE can be considered as such. In that case, there should be a #error in bits/endian.h, or a configure-time error, or something like that, to make clear the glibc port does not support BE.
On 12/18/18 3:11 PM, Joseph Myers wrote: > Another general point: when posting a new port, could you include pointers > to architecture and ABI reference manuals in case those are of relevance > to the review? (URLs going directly to PDFs of those manuals are > preferred.) The PRM (Programmer's Reference Manual) is not open source and per corporate policy requires license agreement, signing NDA... The ABI doc as of today is not open as well, although I'm checking if we could share it and if not create equivalent content like some of the other guys on github etc. Thx for your understanding. -Vineet
On Wed, 19 Dec 2018, Vineet Gupta wrote: > On 12/18/18 3:11 PM, Joseph Myers wrote: > > Another general point: when posting a new port, could you include pointers > > to architecture and ABI reference manuals in case those are of relevance > > to the review? (URLs going directly to PDFs of those manuals are > > preferred.) > > The PRM (Programmer's Reference Manual) is not open source and per corporate > policy requires license agreement, signing NDA... It's very questionable whether we should consider a port for inclusion in glibc at all without public architecture documentation (that is, documentation of instruction semantics; not necessarily documentation of microarchitectural performance characteristics etc.). Various kinds of changes can require a developer to refer to documentation across the range of architectures supported by glibc, if something requires assembly implementations across architectures (e.g. some of Adhemerval's changes to thread cancellation; or when I added fegetmode / fesetmode, that required referring to various architecture documentation to identify what bits should be considered mode bits, on each architecture where floating-point status and control bits occupy a single register). (Non-NDA click-throughs, like the Arm one agreeing not to use the manual to find if Arm implementations violate any patents, are OK, presuming there is nothing to restrict public discussion of the contents and using the information therein to develop free software, although having a direct URL to the manual without needing such a click-through is certainly preferred.)
On 12/19/18 4:14 PM, Joseph Myers wrote: > On Wed, 19 Dec 2018, Vineet Gupta wrote: > >> On 12/18/18 3:11 PM, Joseph Myers wrote: >>> Another general point: when posting a new port, could you include pointers >>> to architecture and ABI reference manuals in case those are of relevance >>> to the review? (URLs going directly to PDFs of those manuals are >>> preferred.) >> >> The PRM (Programmer's Reference Manual) is not open source and per corporate >> policy requires license agreement, signing NDA... > > It's very questionable whether we should consider a port for inclusion in > glibc at all without public architecture documentation IMHO this seems excessive. We've successfully open-sourced significant ports such as Linux kernel, gcc, binutils etc. gcc atleast deals with instruction semantics at a much closer level than glibc perse and that didn't get in the way then. Being an open source developer myself I agree with your reservations, but the reality is what Ted Tso so nicely put in at [1] | "It's something I do worry about; and I do share your concern. At the | same time, the reality is that we are a little like the Old Dutch | Masters, who had take into account the preference of their patrons | (i.e., in our case, those who pay our paychecks :-)" [1] https://lwn.net/Articles/446626/ > (that is, > documentation of instruction semantics; not necessarily documentation of > microarchitectural performance characteristics etc.). I understand the intent is all good faith and justified... > Various kinds of > changes can require a developer to refer to documentation across the range > of architectures supported by glibc, if something requires assembly > implementations across architectures (e.g. some of Adhemerval's changes to > thread cancellation; or when I added fegetmode / fesetmode, that required > referring to various architecture documentation to identify what bits > should be considered mode bits, on each architecture where floating-point > status and control bits occupy a single register). To be honest folks on lkml do sweeping arch changes, PeterZ is one good example and he has infact changed ARC assembly at times w/o access to PRM. Given the arrangement we have now, perhaps such changes can call in for review from port maintainer etc. > (Non-NDA click-throughs, like the Arm one agreeing not to use the manual > to find if Arm implementations violate any patents, are OK No that is not possible: I've discussed this with power may-be in the past and again today and this is not going to happen. Thx, -Vineet
On 12/18/18 1:04 PM, Vineet Gupta wrote: > (a) build-many-glibcs.py check results are clean > > | Summary of test results: > | 1173 PASS > | 15 XFAIL > | > | PASS: glibcs-arc-linux-gnu check One of the issues in running build-many incrementally, i.e. "host-libraries", "compilers" only once but "glibcs" again to quickly test fixes is that for 2nd run, target c++ compiler is picked up for support/links-dso-program vs. links-dso-program-c which fails as the cpp headers are not present. Is there a solution to that ? > (b) Full testsuite ran in a cross compile setup using buildroot on HSDK development > platform. > > Summary of test results: > 24 FAIL > 5124 PASS > 27 UNSUPPORTED > 19 XFAIL One of the pesky issues with cross testing is localedata/gen-locale.sh kicking on target, which despite being 1GHz gets in the way of quick iterations. Is there a way to build those on host and simply reference/install them on target. I understand this is more of a buildsystem question (buildroot), but any pointers would be appreciated.
On Thu, 20 Dec 2018, Vineet Gupta wrote: > On 12/18/18 1:04 PM, Vineet Gupta wrote: > > (a) build-many-glibcs.py check results are clean > > > > | Summary of test results: > > | 1173 PASS > > | 15 XFAIL > > | > > | PASS: glibcs-arc-linux-gnu check > > One of the issues in running build-many incrementally, i.e. "host-libraries", > "compilers" only once but "glibcs" again to quickly test fixes is that for 2nd > run, target c++ compiler is picked up for support/links-dso-program vs. > links-dso-program-c which fails as the cpp headers are not present. Is there a > solution to that ? I'm not aware of that problem. The C++ headers certainly should be available in the compilers installed by the "compilers" step. My bots using GCC 7 and 8 branches only rebuild the compilers once a week unless build-many-glibcs.py itself changes. > One of the pesky issues with cross testing is localedata/gen-locale.sh > kicking on target, which despite being 1GHz gets in the way of quick > iterations. Is there a way to build those on host and simply > reference/install them on target. I understand this is more of a > buildsystem question (buildroot), but any pointers would be appreciated. It's possible to set up an external build system that extracts the list of locales for testing from localedata/Makefile, builds them using the same version of localedef built for the build system and appropriate options for endianness and uint32_t alignment (if necessary) and copies them into the right places in the glibc build tree before running the glibc tests. It does have to be the same version of localedef since the binary locale format can change depending on the glibc version.
On 12/18/18 3:52 PM, Joseph Myers wrote: > On Tue, 18 Dec 2018, Vineet Gupta wrote: > >> At any rate, the generated code will likely be different for -Os and otherwise, >> does that mean entries get added conditionally to localplt.data etc ? > > Entries *can* have a "?" to indicate they are optional OK. > (although avoiding > the PLT reference is better - see how include/string.h does asm > redirection of mempcpy and stpcpy, or likewise in > sysdeps/wordsize-32/divdi3-symbol-hacks.h, for example). In libc.so, the extra PLT entry for memcpy | 00106034 00047837 R_ARC_JMP_SLOT 00066d5c memcpy@@GLIBC_2.27 + 0 | ... | 1bec8: ld r12,[pcl,0xea16c] ;106034 <memcpy@@GLIBC_2.27+0x9f2d8> | 1bed0: j.d [r12] | 1bed4: mov r12,pcl is due to linkage of a generic libgcc routine fp-bit.c | 000d79c4 <_fpadd_parts>: |... | d7a1a: bl -768848 ;1bec8 <.plt+0xd0> | that in turn is likely due to struct assignment in the routine |static const fp_number_type * |_fpadd_parts (fp_number_type * a, | fp_number_type * b, | fp_number_type * tmp) |{ | ... | if (iszero (a)) | { | *tmp = *a; So this is classic case of gcc -Os in action, generating a memcpy vs. in-place LD/ST based copy. So I guess we just add "?" to localplt.data >> ARC historically has supported both LE/BE, but we are kind of moving >> away from BE and as far as I know, there's no-one actively working with >> BE (except 1 specific customer with legact cores). It is safe to assume >> we won't support BE and this is sort of easier said for ARC processors >> as they have many many hardware config knobs and not all are supported >> for Linux usecase, so BE can be considered as such. > > In that case, there should be a #error in bits/endian.h, or a > configure-time error, or something like that, to make clear the glibc port > does not support BE. OK, I'll add that.
On Fri, 21 Dec 2018, Vineet Gupta wrote:
> is due to linkage of a generic libgcc routine fp-bit.c
I'd suggest moving from fp-bit to soft-fp in libgcc (soft-fp is a lot
faster, see the 2006 GCC Summit proceedings, though of course you may wish
to do your own benchmarking on ARC), which would probably avoid that issue
(though if very concerned about about code size, soft-fp is a bit larger).
But the "?" in localplt.data would still make sense as long as the GCC
versions using fp-bit there are supported for building the ARC glibc port.
On 12/21/18 6:32 AM, Joseph Myers wrote: > On Fri, 21 Dec 2018, Vineet Gupta wrote: > >> is due to linkage of a generic libgcc routine fp-bit.c > > I'd suggest moving from fp-bit to soft-fp in libgcc (soft-fp is a lot > faster, see the 2006 GCC Summit proceedings, though of course you may wish > to do your own benchmarking on ARC), which would probably avoid that issue It seems the we have "assisted" soft-fp: hand tweaked assembler written originally by Joern, so he and/or Claudiu can advise on that. Although the asm was tweaked for prev core/micro-architecture it should likely be faster than generic "C" code I presume. > (though if very concerned about about code size, soft-fp is a bit larger). Not for glibc :-) (uClibc is a different story, but it seems with largers RAMs and disks size is kind of becoming moot unless deeply embedded. > But the "?" in localplt.data would still make sense as long as the GCC > versions using fp-bit there are supported for building the ARC glibc port. Right, added that already.