| Message ID | 5044DB07.4010005@gjlay.de |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 3, 2012 at 11:29 AM, Georg-Johann Lay <avr@gjlay.de> wrote: > AVR-Libc comes with hand-optimized float support functions written > in assembler. These functions use the same naming conventions like > libgcc. There are situations where this name clashed lead to performance > regression because the functions from libgcc are linked. One example > are the new fixed-point support that convert fixed-point to/from float > and reference float/int conversion functions from within libgcc. > > The float implementation in libm.a have been discussed several times > with the only result that it is very unlikely that the code will > ever be integrated into libgcc because the original authors are no > more around. And is is much less work to add a new configure switch > than to port and integrate the code, given there were no license issues. > One point against such an extension was that such change to the compiler > establishes a dependency between the compiler and AVR-Libc, but this > decision has been made long ago by accepting code that actually should > had been added to libgcc -- but was not for whatever reason. > > This patch removes that performance regressions by removing the > doubly implemented functions from libgcc by means of a new configure > option --with-avrlibc. Johann, as I stated yesterday, I do not understand why there needs to be yet another configure option. The NATURAL libc for ARV targets is ARV-libc. We should not need a switch for that. -- Gaby > > Moreover, some specs are adjusted so that -lm is treated very much like > -lgcc so that the user need not specify -lm by hand for core float > support like int/float conversions. > > Without this patch, LTO compilations also lead to performance regression > because lto adds -plugin-opt=-pass-through=-lgcc etc. prior to the -lm > specified by the user. > > Other cases where code is improved are C++ programs, see PR28718 for a > discussion. > > > There are less fails in gcc.dg/fixed-point, presumably because the rounding > is as expected by the test cases, i.e. there are no rounding errors as > mentioned in the review for PR54222: > http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01586.html > > Ok to install? > > > Johann > > > PR target/54461 > * configure.ac (noconfigdirs,target=avr-*-*): Add target-newlib, > target-libgloss if configured --with-avrlibc. > * configure: Regenerate. > > libgcc/ > PR target/54461 > * config.host (tmake_file,host=avr-*-*): Add avr/t-avrlibc if > configured --with-avrlibc. > * config/avr/t-avrlibc: New file. > > gcc/ > PR target/54461 > * config.gcc (tm_file,target=avr-*-*): Add avr/avrlibc.h if > configured --with-avrlibc. > (tm_defines,target=avr-*-*): Add WITH_AVRLIBC if configured > --with-avrlibc. > * config/avr/avrlibc.h: New file. > * config/avr/avr-c.c: Build-in define __WITH_AVRLIBC__ if > configured --with-avrlibc.
Gabriel Dos Reis schrieb: > On Mon, Sep 3, 2012 at 11:29 AM, Georg-Johann Lay <avr@gjlay.de> wrote: >> AVR-Libc comes with hand-optimized float support functions written >> in assembler. These functions use the same naming conventions like >> libgcc. There are situations where this name clashed lead to performance >> regression because the functions from libgcc are linked. One example >> are the new fixed-point support that convert fixed-point to/from float >> and reference float/int conversion functions from within libgcc. >> >> The float implementation in libm.a have been discussed several times >> with the only result that it is very unlikely that the code will >> ever be integrated into libgcc because the original authors are no >> more around. And is is much less work to add a new configure switch >> than to port and integrate the code, given there were no license issues. >> One point against such an extension was that such change to the compiler >> establishes a dependency between the compiler and AVR-Libc, but this >> decision has been made long ago by accepting code that actually should >> had been added to libgcc -- but was not for whatever reason. >> >> This patch removes that performance regressions by removing the >> doubly implemented functions from libgcc by means of a new configure >> option --with-avrlibc. > > Johann, > > as I stated yesterday, I do not understand why there needs to be yet another > configure option. The NATURAL libc for ARV targets is ARV-libc. We > should not need a switch for that. There is also newlib that is used with avr-gcc. I know this because some bugs are only triggered for newlib. If there are users that report bugs if avr-gcc is build for newlib, I'd guess these users are actually interested in using newlib. It's clear that the proposed changes do *not* work with newlib because newlib does not mimic parts of libgcc. Johann >> Moreover, some specs are adjusted so that -lm is treated very much like >> -lgcc so that the user need not specify -lm by hand for core float >> support like int/float conversions. >> >> Without this patch, LTO compilations also lead to performance regression >> because lto adds -plugin-opt=-pass-through=-lgcc etc. prior to the -lm >> specified by the user. >> >> Other cases where code is improved are C++ programs, see PR28718 for a >> discussion. >> >> >> There are less fails in gcc.dg/fixed-point, presumably because the rounding >> is as expected by the test cases, i.e. there are no rounding errors as >> mentioned in the review for PR54222: >> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01586.html >> >> Ok to install? >> >> >> Johann >> >> >> PR target/54461 >> * configure.ac (noconfigdirs,target=avr-*-*): Add target-newlib, >> target-libgloss if configured --with-avrlibc. >> * configure: Regenerate. >> >> libgcc/ >> PR target/54461 >> * config.host (tmake_file,host=avr-*-*): Add avr/t-avrlibc if >> configured --with-avrlibc. >> * config/avr/t-avrlibc: New file. >> >> gcc/ >> PR target/54461 >> * config.gcc (tm_file,target=avr-*-*): Add avr/avrlibc.h if >> configured --with-avrlibc. >> (tm_defines,target=avr-*-*): Add WITH_AVRLIBC if configured >> --with-avrlibc. >> * config/avr/avrlibc.h: New file. >> * config/avr/avr-c.c: Build-in define __WITH_AVRLIBC__ if >> configured --with-avrlibc. >
On Mon, Sep 3, 2012 at 1:46 PM, Georg-Johann Lay <avr@gjlay.de> wrote: > Gabriel Dos Reis schrieb: > >> On Mon, Sep 3, 2012 at 11:29 AM, Georg-Johann Lay <avr@gjlay.de> wrote: >>> >>> AVR-Libc comes with hand-optimized float support functions written >>> in assembler. These functions use the same naming conventions like >>> libgcc. There are situations where this name clashed lead to performance >>> regression because the functions from libgcc are linked. One example >>> are the new fixed-point support that convert fixed-point to/from float >>> and reference float/int conversion functions from within libgcc. >>> >>> The float implementation in libm.a have been discussed several times >>> with the only result that it is very unlikely that the code will >>> ever be integrated into libgcc because the original authors are no >>> more around. And is is much less work to add a new configure switch >>> than to port and integrate the code, given there were no license issues. >>> One point against such an extension was that such change to the compiler >>> establishes a dependency between the compiler and AVR-Libc, but this >>> decision has been made long ago by accepting code that actually should >>> had been added to libgcc -- but was not for whatever reason. >>> >>> This patch removes that performance regressions by removing the >>> doubly implemented functions from libgcc by means of a new configure >>> option --with-avrlibc. >> >> >> Johann, >> >> as I stated yesterday, I do not understand why there needs to be yet >> another >> configure option. The NATURAL libc for ARV targets is ARV-libc. We >> should not need a switch for that. > > > There is also newlib that is used with avr-gcc. I know this because > some bugs are only triggered for newlib. If there are users that > report bugs if avr-gcc is build for newlib, I'd guess these users are > actually interested in using newlib. I did not say there was no other libc library. I said that the *natural* libc appears to be AVR-libc. We don't configure GCC/g++ saying --with-libstdc++. -- Gaby > > It's clear that the proposed changes do *not* work with newlib > because newlib does not mimic parts of libgcc. > > Johann > > >>> Moreover, some specs are adjusted so that -lm is treated very much like >>> -lgcc so that the user need not specify -lm by hand for core float >>> support like int/float conversions. >>> >>> Without this patch, LTO compilations also lead to performance regression >>> because lto adds -plugin-opt=-pass-through=-lgcc etc. prior to the -lm >>> specified by the user. >>> >>> Other cases where code is improved are C++ programs, see PR28718 for a >>> discussion. >>> >>> >>> There are less fails in gcc.dg/fixed-point, presumably because the >>> rounding >>> is as expected by the test cases, i.e. there are no rounding errors as >>> mentioned in the review for PR54222: >>> http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01586.html >>> >>> Ok to install? >>> >>> >>> Johann >>> >>> >>> PR target/54461 >>> * configure.ac (noconfigdirs,target=avr-*-*): Add target-newlib, >>> target-libgloss if configured --with-avrlibc. >>> * configure: Regenerate. >>> >>> libgcc/ >>> PR target/54461 >>> * config.host (tmake_file,host=avr-*-*): Add avr/t-avrlibc if >>> configured --with-avrlibc. >>> * config/avr/t-avrlibc: New file. >>> >>> gcc/ >>> PR target/54461 >>> * config.gcc (tm_file,target=avr-*-*): Add avr/avrlibc.h if >>> configured --with-avrlibc. >>> (tm_defines,target=avr-*-*): Add WITH_AVRLIBC if configured >>> --with-avrlibc. >>> * config/avr/avrlibc.h: New file. >>> * config/avr/avr-c.c: Build-in define __WITH_AVRLIBC__ if >>> configured --with-avrlibc. >> >> >
Gabriel Dos Reis schrieb: > Georg-Johann Lay wrote: >> Gabriel Dos Reis schrieb: >>> Georg-Johann Lay wrote: >>>> AVR-Libc comes with hand-optimized float support functions written >>>> in assembler. These functions use the same naming conventions like >>>> libgcc. There are situations where this name clashed lead to performance >>>> regression because the functions from libgcc are linked. One example >>>> are the new fixed-point support that convert fixed-point to/from float >>>> and reference float/int conversion functions from within libgcc. >>>> >>>> The float implementation in libm.a have been discussed several times >>>> with the only result that it is very unlikely that the code will >>>> ever be integrated into libgcc because the original authors are no >>>> more around. And is is much less work to add a new configure switch >>>> than to port and integrate the code, given there were no license issues. >>>> One point against such an extension was that such change to the compiler >>>> establishes a dependency between the compiler and AVR-Libc, but this >>>> decision has been made long ago by accepting code that actually should >>>> had been added to libgcc -- but was not for whatever reason. >>>> >>>> This patch removes that performance regressions by removing the >>>> doubly implemented functions from libgcc by means of a new configure >>>> option --with-avrlibc. >>> >>> as I stated yesterday, I do not understand why there needs to be yet >>> another >>> configure option. The NATURAL libc for ARV targets is ARV-libc. We >>> should not need a switch for that. >> >> There is also newlib that is used with avr-gcc. I know this because >> some bugs are only triggered for newlib. If there are users that >> report bugs if avr-gcc is build for newlib, I'd guess these users are >> actually interested in using newlib. > > I did not say there was no other libc library. I said > that the *natural* libc appears to be AVR-libc. > > We don't configure GCC/g++ saying --with-libstdc++. That's a different story because these libraries support in-tree build just like newlib does. This is not true for AVR-Libc which does not support in-tree builds. I agree that AVR-Libc is the most common libc implementation used with avr-gcc and is has many advantages over other libc implementation (except that it does not support in-tree builds). However, a --with-avrlibc is not needed to *get* the support from AVR-Libc, it's just used to fix some problems that arise in certain use cases. Besides that, the proposed arrangement does not affect the configuration if the switch is *not* specified, thus the patch is appropriate to be backported. My intention is to backport it to 4.7 as indicated by the milestone, but if the change was unconditionally I don't think the change is appropriate for a backport. And after all it's just a *configure* option that some distribution maintainers can set if they want to. The tool chain user is not bothered at all by the new option and won't even notice it. From the user perspective it's just as if some optimizations had been added to the tool chain. What do you propose? Use the setting per default and support a --with-avrlibc=no if the user want full libgcc support and nothing removed from it? >> It's clear that the proposed changes do *not* work with newlib >> because newlib does not mimic parts of libgcc. >> >> Johann
As Georg-Johann Lay wrote:
> What do you propose?
I'm fine with that option, and think it's a good idea.
2012/9/4 Joerg Wunsch <j@uriah.heep.sax.de>: > As Georg-Johann Lay wrote: > >> What do you propose? > > I'm fine with that option, and think it's a good idea. > I have not objections against the patch. Denis
On Mon, Sep 3, 2012 at 4:23 PM, Georg-Johann Lay <avr@gjlay.de> wrote: > Gabriel Dos Reis schrieb: >> >> Georg-Johann Lay wrote: >>> >>> Gabriel Dos Reis schrieb: >>>> >>>> Georg-Johann Lay wrote: >>>>> >>>>> AVR-Libc comes with hand-optimized float support functions written >>>>> in assembler. These functions use the same naming conventions like >>>>> libgcc. There are situations where this name clashed lead to >>>>> performance >>>>> regression because the functions from libgcc are linked. One example >>>>> are the new fixed-point support that convert fixed-point to/from float >>>>> and reference float/int conversion functions from within libgcc. >>>>> >>>>> The float implementation in libm.a have been discussed several times >>>>> with the only result that it is very unlikely that the code will >>>>> ever be integrated into libgcc because the original authors are no >>>>> more around. And is is much less work to add a new configure switch >>>>> than to port and integrate the code, given there were no license >>>>> issues. >>>>> One point against such an extension was that such change to the >>>>> compiler >>>>> establishes a dependency between the compiler and AVR-Libc, but this >>>>> decision has been made long ago by accepting code that actually should >>>>> had been added to libgcc -- but was not for whatever reason. >>>>> >>>>> This patch removes that performance regressions by removing the >>>>> doubly implemented functions from libgcc by means of a new configure >>>>> option --with-avrlibc. >>>> >>>> >>>> as I stated yesterday, I do not understand why there needs to be yet >>>> another >>>> configure option. The NATURAL libc for ARV targets is ARV-libc. We >>>> should not need a switch for that. >>> >>> >>> There is also newlib that is used with avr-gcc. I know this because >>> some bugs are only triggered for newlib. If there are users that >>> report bugs if avr-gcc is build for newlib, I'd guess these users are >>> actually interested in using newlib. >> >> >> I did not say there was no other libc library. I said >> that the *natural* libc appears to be AVR-libc. >> >> We don't configure GCC/g++ saying --with-libstdc++. > > > That's a different story because these libraries support in-tree > build just like newlib does. This is not true for AVR-Libc which > does not support in-tree builds. > > I agree that AVR-Libc is the most common libc implementation > used with avr-gcc and is has many advantages over other libc > implementation (except that it does not support in-tree builds). I think the "in-tree builds" thing is a red herring. > > However, a --with-avrlibc is not needed to *get* the support > from AVR-Libc, it's just used to fix some problems that arise > in certain use cases. so, let's make it the default -- see below. > > Besides that, the proposed arrangement does not affect the > configuration if the switch is *not* specified, thus the patch > is appropriate to be backported. > > My intention is to backport it to 4.7 as indicated by the milestone, > but if the change was unconditionally I don't think the change is > appropriate for a backport. > It is perfectly reasonable and OK to to make the backport more guarded (e.g. by the configure option) than on mainline. > And after all it's just a *configure* option that some distribution > maintainers can set if they want to. yes, but it is still one more configure option. > The tool chain user is not > bothered at all by the new option and won't even notice it. > From the user perspective it's just as if some optimizations > had been added to the tool chain. > > What do you propose? > > Use the setting per default and support a --with-avrlibc=no if > the user want full libgcc support and nothing removed from it? Yes. Let's make the "sane" behaviour the default. -- Gaby
Gabriel Dos Reis schrieb: > Georg-Johann Lay wrote: >> Gabriel Dos Reis schrieb: >>> Georg-Johann Lay wrote: >>>> Gabriel Dos Reis schrieb: >>>>> Georg-Johann Lay wrote: >>>>>> AVR-Libc comes with hand-optimized float support functions >>>>>> written in assembler. These functions use the same naming >>>>>> conventions like libgcc. There are situations where this >>>>>> name clashed lead to performance regression because the >>>>>> functions from libgcc are linked. One example are the new >>>>>> fixed-point support that convert fixed-point to/from float >>>>>> and reference float/int conversion functions from within >>>>>> libgcc. >>>>>> >>>>>> The float implementation in libm.a have been discussed >>>>>> several times with the only result that it is very unlikely >>>>>> that the code will ever be integrated into libgcc because >>>>>> the original authors are no more around. And is is much >>>>>> less work to add a new configure switch than to port and >>>>>> integrate the code, given there were no license issues. One >>>>>> point against such an extension was that such change to the >>>>>> compiler establishes a dependency between the compiler and >>>>>> AVR-Libc, but this decision has been made long ago by >>>>>> accepting code that actually should had been added to >>>>>> libgcc -- but was not for whatever reason. >>>>>> >>>>>> This patch removes that performance regressions by removing >>>>>> the doubly implemented functions from libgcc by means of a >>>>>> new configure option --with-avrlibc. >>>>> >>>>> as I stated yesterday, I do not understand why there needs to >>>>> be yet another configure option. The NATURAL libc for ARV >>>>> targets is ARV-libc. We should not need a switch for that. >>>> >>>> There is also newlib that is used with avr-gcc. I know this >>>> because some bugs are only triggered for newlib. If there are >>>> users that report bugs if avr-gcc is build for newlib, I'd >>>> guess these users are actually interested in using newlib. >>> >>> I did not say there was no other libc library. I said that the >>> *natural* libc appears to be AVR-libc. >>> >>> We don't configure GCC/g++ saying --with-libstdc++. >> >> That's a different story because these libraries support in-tree >> build just like newlib does. This is not true for AVR-Libc which >> does not support in-tree builds. >> >> I agree that AVR-Libc is the most common libc implementation used >> with avr-gcc and is has many advantages over other libc >> implementation (except that it does not support in-tree builds). > > I think the "in-tree builds" thing is a red herring. I don't think so. If there was an in-tree build gcc could detect itself whether or not AVR-Libc is present or not. With the current setup the user has to specify that -- in whatever direction: that libc is there or that libc is not there depending on whatever is default. >> However, a --with-avrlibc is not needed to *get* the support from >> AVR-Libc, it's just used to fix some problems that arise in certain >> use cases. > > so, let's make it the default -- see below. > >> Besides that, the proposed arrangement does not affect the >> configuration if the switch is *not* specified, thus the patch is >> appropriate to be backported. >> >> My intention is to backport it to 4.7 as indicated by the >> milestone, but if the change was unconditionally I don't think the >> change is appropriate for a backport. > > It is perfectly reasonable and OK to to make the backport more > guarded (e.g. by the configure option) than on mainline. > >> And after all it's just a *configure* option that some distribution >> maintainers can set if they want to. > > yes, but it is still one more configure option. hmm. The configure machinery was not changed, it automatically sets with_foo if --with-foo is specified. It's just about who is to be blamed if he does not read the release notes ;-) Whatever, I think we two are stuck now and enough arguments passed back and forth. Let the port maintainers decide. And Jörg, would you check the excludes list in t-avrlibc? Johann >> The tool chain user is not bothered at all by the new option and >> won't even notice it. From the user perspective it's just as if >> some optimizations had been added to the tool chain. >> >> What do you propose? >> >> Use the setting per default and support a --with-avrlibc=no if the >> user want full libgcc support and nothing removed from it? > > Yes. Let's make the "sane" behaviour the default. > > -- Gaby
On Tue, Sep 4, 2012 at 1:55 AM, Georg-Johann Lay <avr@gjlay.de> wrote: > Gabriel Dos Reis schrieb: >> >> Georg-Johann Lay wrote: >>> >>> Gabriel Dos Reis schrieb: >>>> >>>> Georg-Johann Lay wrote: >>>>> >>>>> Gabriel Dos Reis schrieb: >>>>>> >>>>>> Georg-Johann Lay wrote: >>>>>>> >>>>>>> AVR-Libc comes with hand-optimized float support functions >>>>>>> written in assembler. These functions use the same naming >>>>>>> conventions like libgcc. There are situations where this >>>>>>> name clashed lead to performance regression because the >>>>>>> functions from libgcc are linked. One example are the new >>>>>>> fixed-point support that convert fixed-point to/from float and >>>>>>> reference float/int conversion functions from within >>>>>>> libgcc. >>>>>>> >>>>>>> The float implementation in libm.a have been discussed >>>>>>> several times with the only result that it is very unlikely >>>>>>> that the code will ever be integrated into libgcc because >>>>>>> the original authors are no more around. And is is much >>>>>>> less work to add a new configure switch than to port and >>>>>>> integrate the code, given there were no license issues. One >>>>>>> point against such an extension was that such change to the >>>>>>> compiler establishes a dependency between the compiler and >>>>>>> AVR-Libc, but this decision has been made long ago by >>>>>>> accepting code that actually should had been added to >>>>>>> libgcc -- but was not for whatever reason. >>>>>>> >>>>>>> This patch removes that performance regressions by removing >>>>>>> the doubly implemented functions from libgcc by means of a >>>>>>> new configure option --with-avrlibc. >>>>>> >>>>>> >>>>>> as I stated yesterday, I do not understand why there needs to >>>>>> be yet another configure option. The NATURAL libc for ARV >>>>>> targets is ARV-libc. We should not need a switch for that. >>>>> >>>>> >>>>> There is also newlib that is used with avr-gcc. I know this >>>>> because some bugs are only triggered for newlib. If there are >>>>> users that report bugs if avr-gcc is build for newlib, I'd >>>>> guess these users are actually interested in using newlib. >>>> >>>> >>>> I did not say there was no other libc library. I said that the >>>> *natural* libc appears to be AVR-libc. >>>> >>>> We don't configure GCC/g++ saying --with-libstdc++. >>> >>> >>> That's a different story because these libraries support in-tree build >>> just like newlib does. This is not true for AVR-Libc which does not support >>> in-tree builds. >>> >>> I agree that AVR-Libc is the most common libc implementation used >>> with avr-gcc and is has many advantages over other libc implementation >>> (except that it does not support in-tree builds). >> >> >> I think the "in-tree builds" thing is a red herring. > > > I don't think so. If there was an in-tree build gcc could detect > itself whether or not AVR-Libc is present or not. With the > current setup the user has to specify that -- in whatever > direction: that libc is there or that libc is not there depending > on whatever is default. obviously that situation isn't ideal, and we shouldn't build patches that are as if it it should be perpetuated. [...] >> yes, but it is still one more configure option. > > > hmm. The configure machinery was not changed, It is one more configure option for user to specify, no matter how the internal configury is implemented. -- Gaby
On Tue, Sep 4, 2012 at 8:55 AM, Georg-Johann Lay <avr@gjlay.de> wrote: > Gabriel Dos Reis schrieb: >> >> Georg-Johann Lay wrote: >>> >>> Gabriel Dos Reis schrieb: >>>> >>>> Georg-Johann Lay wrote: >>>>> >>>>> Gabriel Dos Reis schrieb: >>>>>> >>>>>> Georg-Johann Lay wrote: >>>>>>> >>>>>>> AVR-Libc comes with hand-optimized float support functions >>>>>>> written in assembler. These functions use the same naming >>>>>>> conventions like libgcc. There are situations where this >>>>>>> name clashed lead to performance regression because the >>>>>>> functions from libgcc are linked. One example are the new >>>>>>> fixed-point support that convert fixed-point to/from float and >>>>>>> reference float/int conversion functions from within >>>>>>> libgcc. >>>>>>> >>>>>>> The float implementation in libm.a have been discussed >>>>>>> several times with the only result that it is very unlikely >>>>>>> that the code will ever be integrated into libgcc because >>>>>>> the original authors are no more around. And is is much >>>>>>> less work to add a new configure switch than to port and >>>>>>> integrate the code, given there were no license issues. One >>>>>>> point against such an extension was that such change to the >>>>>>> compiler establishes a dependency between the compiler and >>>>>>> AVR-Libc, but this decision has been made long ago by >>>>>>> accepting code that actually should had been added to >>>>>>> libgcc -- but was not for whatever reason. >>>>>>> >>>>>>> This patch removes that performance regressions by removing >>>>>>> the doubly implemented functions from libgcc by means of a >>>>>>> new configure option --with-avrlibc. >>>>>> >>>>>> >>>>>> as I stated yesterday, I do not understand why there needs to >>>>>> be yet another configure option. The NATURAL libc for ARV >>>>>> targets is ARV-libc. We should not need a switch for that. >>>>> >>>>> >>>>> There is also newlib that is used with avr-gcc. I know this >>>>> because some bugs are only triggered for newlib. If there are >>>>> users that report bugs if avr-gcc is build for newlib, I'd >>>>> guess these users are actually interested in using newlib. >>>> >>>> >>>> I did not say there was no other libc library. I said that the >>>> *natural* libc appears to be AVR-libc. >>>> >>>> We don't configure GCC/g++ saying --with-libstdc++. >>> >>> >>> That's a different story because these libraries support in-tree build >>> just like newlib does. This is not true for AVR-Libc which does not support >>> in-tree builds. >>> >>> I agree that AVR-Libc is the most common libc implementation used >>> with avr-gcc and is has many advantages over other libc implementation >>> (except that it does not support in-tree builds). >> >> >> I think the "in-tree builds" thing is a red herring. > > > I don't think so. If there was an in-tree build gcc could detect > itself whether or not AVR-Libc is present or not. With the > current setup the user has to specify that -- in whatever > direction: that libc is there or that libc is not there depending > on whatever is default. You can do a link check on whether -lc provides those functions and skip those that overlap with libgcc. Richard. > >>> However, a --with-avrlibc is not needed to *get* the support from >>> AVR-Libc, it's just used to fix some problems that arise in certain >>> use cases. >> >> >> so, let's make it the default -- see below. >> >>> Besides that, the proposed arrangement does not affect the configuration >>> if the switch is *not* specified, thus the patch is >>> appropriate to be backported. >>> >>> My intention is to backport it to 4.7 as indicated by the >>> milestone, but if the change was unconditionally I don't think the >>> change is appropriate for a backport. >> >> >> It is perfectly reasonable and OK to to make the backport more guarded >> (e.g. by the configure option) than on mainline. >> >>> And after all it's just a *configure* option that some distribution >>> maintainers can set if they want to. >> >> >> yes, but it is still one more configure option. > > > hmm. The configure machinery was not changed, it automatically sets > with_foo if --with-foo is specified. It's just about who is to > be blamed if he does not read the release notes ;-) > > Whatever, I think we two are stuck now and enough arguments passed > back and forth. Let the port maintainers decide. > > And Jörg, would you check the excludes list in t-avrlibc? > > Johann > > >>> The tool chain user is not bothered at all by the new option and >>> won't even notice it. From the user perspective it's just as if >>> some optimizations had been added to the tool chain. >>> >>> What do you propose? >>> >>> Use the setting per default and support a --with-avrlibc=no if the >>> user want full libgcc support and nothing removed from it? >> >> >> Yes. Let's make the "sane" behaviour the default. >> >> -- Gaby > >
Richard Guenther wrote: > Georg-Johann Lay wrote: >> Gabriel Dos Reis schrieb: >>> Georg-Johann Lay wrote: >>>> Gabriel Dos Reis schrieb: >>>>> Georg-Johann Lay wrote: >>>>>> Gabriel Dos Reis schrieb: >>>>>>> Georg-Johann Lay wrote: >>>>>>>> AVR-Libc comes with hand-optimized float support functions >>>>>>>> written in assembler. These functions use the same naming >>>>>>>> conventions like libgcc. There are situations where this >>>>>>>> name clashed lead to performance regression because the >>>>>>>> functions from libgcc are linked. One example are the new >>>>>>>> fixed-point support that convert fixed-point to/from float and >>>>>>>> reference float/int conversion functions from within >>>>>>>> libgcc. >>>>>>>> >>>>>>>> The float implementation in libm.a have been discussed >>>>>>>> several times with the only result that it is very unlikely >>>>>>>> that the code will ever be integrated into libgcc because >>>>>>>> the original authors are no more around. And is is much >>>>>>>> less work to add a new configure switch than to port and >>>>>>>> integrate the code, given there were no license issues. One >>>>>>>> point against such an extension was that such change to the >>>>>>>> compiler establishes a dependency between the compiler and >>>>>>>> AVR-Libc, but this decision has been made long ago by >>>>>>>> accepting code that actually should had been added to >>>>>>>> libgcc -- but was not for whatever reason. >>>>>>>> >>>>>>>> This patch removes that performance regressions by removing >>>>>>>> the doubly implemented functions from libgcc by means of a >>>>>>>> new configure option --with-avrlibc. >>>>>>> >>>>>>> as I stated yesterday, I do not understand why there needs to >>>>>>> be yet another configure option. The NATURAL libc for ARV >>>>>>> targets is ARV-libc. We should not need a switch for that. >>>>>> >>>>>> There is also newlib that is used with avr-gcc. I know this >>>>>> because some bugs are only triggered for newlib. If there are >>>>>> users that report bugs if avr-gcc is build for newlib, I'd >>>>>> guess these users are actually interested in using newlib. >>>>> >>>>> I did not say there was no other libc library. I said that the >>>>> *natural* libc appears to be AVR-libc. >>>>> >>>>> We don't configure GCC/g++ saying --with-libstdc++. >>>> >>>> That's a different story because these libraries support in-tree build >>>> just like newlib does. This is not true for AVR-Libc which does not support >>>> in-tree builds. >>>> >>>> I agree that AVR-Libc is the most common libc implementation used >>>> with avr-gcc and is has many advantages over other libc implementation >>>> (except that it does not support in-tree builds). >>> >>> I think the "in-tree builds" thing is a red herring. >> >> I don't think so. If there was an in-tree build gcc could detect >> itself whether or not AVR-Libc is present or not. With the >> current setup the user has to specify that -- in whatever >> direction: that libc is there or that libc is not there depending >> on whatever is default. > > You can do a link check on whether -lc provides those functions > and skip those that overlap with libgcc. Can you explain this? A typical build of avr tools goes like 1) configure, build and install binutils 2) configure, build and install the compiler 3) configure, build and install AVR-Libc so that in step 2 no checking is possible because there is no -lc yet. Or do you mean a check at run time (of the compiler)? Johann
On Tue, Sep 4, 2012 at 1:01 PM, Georg-Johann Lay <avr@gjlay.de> wrote: > Richard Guenther wrote: >> Georg-Johann Lay wrote: >>> Gabriel Dos Reis schrieb: >>>> Georg-Johann Lay wrote: >>>>> Gabriel Dos Reis schrieb: >>>>>> Georg-Johann Lay wrote: >>>>>>> Gabriel Dos Reis schrieb: >>>>>>>> Georg-Johann Lay wrote: >>>>>>>>> AVR-Libc comes with hand-optimized float support functions >>>>>>>>> written in assembler. These functions use the same naming >>>>>>>>> conventions like libgcc. There are situations where this >>>>>>>>> name clashed lead to performance regression because the >>>>>>>>> functions from libgcc are linked. One example are the new >>>>>>>>> fixed-point support that convert fixed-point to/from float and >>>>>>>>> reference float/int conversion functions from within >>>>>>>>> libgcc. >>>>>>>>> >>>>>>>>> The float implementation in libm.a have been discussed >>>>>>>>> several times with the only result that it is very unlikely >>>>>>>>> that the code will ever be integrated into libgcc because >>>>>>>>> the original authors are no more around. And is is much >>>>>>>>> less work to add a new configure switch than to port and >>>>>>>>> integrate the code, given there were no license issues. One >>>>>>>>> point against such an extension was that such change to the >>>>>>>>> compiler establishes a dependency between the compiler and >>>>>>>>> AVR-Libc, but this decision has been made long ago by >>>>>>>>> accepting code that actually should had been added to >>>>>>>>> libgcc -- but was not for whatever reason. >>>>>>>>> >>>>>>>>> This patch removes that performance regressions by removing >>>>>>>>> the doubly implemented functions from libgcc by means of a >>>>>>>>> new configure option --with-avrlibc. >>>>>>>> >>>>>>>> as I stated yesterday, I do not understand why there needs to >>>>>>>> be yet another configure option. The NATURAL libc for ARV >>>>>>>> targets is ARV-libc. We should not need a switch for that. >>>>>>> >>>>>>> There is also newlib that is used with avr-gcc. I know this >>>>>>> because some bugs are only triggered for newlib. If there are >>>>>>> users that report bugs if avr-gcc is build for newlib, I'd >>>>>>> guess these users are actually interested in using newlib. >>>>>> >>>>>> I did not say there was no other libc library. I said that the >>>>>> *natural* libc appears to be AVR-libc. >>>>>> >>>>>> We don't configure GCC/g++ saying --with-libstdc++. >>>>> >>>>> That's a different story because these libraries support in-tree build >>>>> just like newlib does. This is not true for AVR-Libc which does not support >>>>> in-tree builds. >>>>> >>>>> I agree that AVR-Libc is the most common libc implementation used >>>>> with avr-gcc and is has many advantages over other libc implementation >>>>> (except that it does not support in-tree builds). >>>> >>>> I think the "in-tree builds" thing is a red herring. >>> >>> I don't think so. If there was an in-tree build gcc could detect >>> itself whether or not AVR-Libc is present or not. With the >>> current setup the user has to specify that -- in whatever >>> direction: that libc is there or that libc is not there depending >>> on whatever is default. >> >> You can do a link check on whether -lc provides those functions >> and skip those that overlap with libgcc. > > Can you explain this? A typical build of avr tools goes like > > 1) configure, build and install binutils > 2) configure, build and install the compiler > 3) configure, build and install AVR-Libc > > so that in step 2 no checking is possible because there is no -lc yet. > Or do you mean a check at run time (of the compiler)? 4) build and install the real compiler at which time you have AVR-libc available. AT least that's how you "bootstrap" a glibc cross. Richard. > > Johann > >
On Tue, Sep 4, 2012 at 9:17 AM, Richard Guenther <richard.guenther@gmail.com> wrote: >> Can you explain this? A typical build of avr tools goes like >> >> 1) configure, build and install binutils >> 2) configure, build and install the compiler >> 3) configure, build and install AVR-Libc >> >> so that in step 2 no checking is possible because there is no -lc yet. >> Or do you mean a check at run time (of the compiler)? > > 4) build and install the real compiler > > at which time you have AVR-libc available. AT least that's how you > "bootstrap" a glibc cross. avr-gcc has had a "simplified" build process for a while, as it almost never needed to have a avr-gcc hosted on an avr platform. It is usually built as a cross-compiler that always run on the build platform. What I was suggesting earlier is that we shouldn't continue patching the AVR target as if the current state is almost ideal. Pick a libc -- avr-libc appears to be the natural implementation -- and make it the default as opposed to adding nobs. -- Gaby
> -----Original Message----- > From: dosreis@gmail.com [] On Behalf Of Gabriel Dos > Reis > Sent: Tuesday, September 04, 2012 9:08 AM > To: Richard Guenther > Cc: Georg-Johann Lay; gcc-patches@gcc.gnu.org; Denis Chertykov; Weddington, > Eric; Joerg Wunsch > Subject: Re: [Patch,avr] PR54461: Better AVR-Libc integration > > On Tue, Sep 4, 2012 at 9:17 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: > > >> Can you explain this? A typical build of avr tools goes like > >> > >> 1) configure, build and install binutils > >> 2) configure, build and install the compiler > >> 3) configure, build and install AVR-Libc > >> > >> so that in step 2 no checking is possible because there is no -lc yet. > >> Or do you mean a check at run time (of the compiler)? > > > > 4) build and install the real compiler > > > > at which time you have AVR-libc available. AT least that's how you > > "bootstrap" a glibc cross. > > avr-gcc has had a "simplified" build process for a while, as it almost never > needed to have a avr-gcc hosted on an avr platform. It is usually > built as a cross-compiler that always run on the build platform. > > What I was suggesting earlier is that we shouldn't continue patching > the AVR target as if the current state is almost ideal. Pick a libc -- avr- > libc > appears to be the natural implementation -- and make it the default as > opposed to adding nobs. I also strongly agree with this. AFAIK, the only project that uses newlib as the C library for the AVR target is RTEMS, because, AIUI, they need to have the POSIX interface. The vast majority of AVR users have a toolchain that uses avr-libc. Eric Weddington
Index: configure =================================================================== --- configure (revision 190887) +++ configure (working copy) @@ -3500,6 +3500,13 @@ case "${target}" in arm-*-riscix*) noconfigdirs="$noconfigdirs ld target-libgloss" ;; + avr-*-rtems*) + ;; + avr-*-*) + if test x${with_avrlibc} = xyes; then + noconfigdirs="$noconfigdirs target-newlib target-libgloss" + fi + ;; c4x-*-* | tic4x-*-*) noconfigdirs="$noconfigdirs target-libgloss" ;; Index: configure.ac =================================================================== --- configure.ac (revision 190887) +++ configure.ac (working copy) @@ -891,6 +891,13 @@ case "${target}" in arm-*-riscix*) noconfigdirs="$noconfigdirs ld target-libgloss" ;; + avr-*-rtems*) + ;; + avr-*-*) + if test x${with_avrlibc} = xyes; then + noconfigdirs="$noconfigdirs target-newlib target-libgloss" + fi + ;; c4x-*-* | tic4x-*-*) noconfigdirs="$noconfigdirs target-libgloss" ;; Index: libgcc/config/avr/t-avrlibc =================================================================== --- libgcc/config/avr/t-avrlibc (revision 0) +++ libgcc/config/avr/t-avrlibc (revision 0) @@ -0,0 +1,66 @@ +# This file is used with --with-avrlibc=yes +# +# AVR-Libc comes with hand-optimized float routines. +# For historical reasons, these routines live in AVR-Libc +# and not in libgcc and use the same function names like libgcc. +# To get the best support, i.e. always use the routines from +# AVR-Libc, we remove these routines from libgcc. +# +# See also PR54461. +# +# +# Arithmetic: +# __addsf3 __subsf3 __divsf3 __mulsf3 __negsf2 +# +# Comparison: +# __cmpsf2 __unordsf2 +# __eqsf2 __lesf2 __ltsf2 __nesf2 __gesf2 __gtsf2 +# +# Conversion: +# __fixsfdi __fixunssfdi __floatdisf __floatundisf +# __fixsfsi __fixunssfsi __floatsisf __floatunsisf +# +# +# These functions are contained in modules: +# +# _addsub_sf.o: __addsf3 __subsf3 +# _mul_sf.o: __mulsf3 +# _div_sf.o: __divsf3 +# _negate_sf.o: __negsf2 +# +# _compare_sf.o: __cmpsf2 +# _unord_sf.o: __unordsf2 +# _eq_sf.o: __eqsf2 +# _ne_sf.o: __nesf2 +# _ge_sf.o: __gesf2 +# _gt_sf.o: __gtsf2 +# _le_sf.o: __lesf2 +# _lt_sf.o: __ltsf2 +# +# _fixsfdi.o: __fixsfdi +# _fixunssfdi.o: __fixunssfdi +# _fixunssfsi.o: __fixunssfsi +# _floatdisf.o: __floatdisf +# _floatundisf.o: __floatundisf +# _sf_to_si.o: __fixsfsi +# _si_to_sf.o: __floatsisf +# _usi_to_sf.o: __floatunsisf + + +# SFmode +LIB2FUNCS_EXCLUDE += \ + _addsub_sf \ + _negate_sf \ + _mul_sf _div_sf \ + \ + _compare_sf \ + _unord_sf \ + _eq_sf _ne_sf \ + _gt_sf _ge_sf \ + _lt_sf _le_sf \ + \ + _si_to_sf _sf_to_si \ + _usi_to_sf _sf_to_usi \ + _fixunssfsi _fixsfdi \ + _fixunssfdi \ + _floatdisf _floatundisf Index: libgcc/config.host =================================================================== --- libgcc/config.host (revision 190873) +++ libgcc/config.host (working copy) @@ -380,6 +380,9 @@ avr-*-rtems*) avr-*-*) # Make HImode functions for AVR tmake_file="${cpu_type}/t-avr t-fpbit" + if test x${with_avrlibc} = xyes; then + tmake_file="$tmake_file ${cpu_type}/t-avrlibc" + fi tm_file="$tm_file avr/avr-lib.h" ;; bfin*-elf*) Index: gcc/config/avr/avr-c.c =================================================================== --- gcc/config/avr/avr-c.c (revision 190873) +++ gcc/config/avr/avr-c.c (working copy) @@ -148,6 +148,10 @@ avr_cpu_cpp_builtins (struct cpp_reader cpp_define_formatted (pfile, "__AVR_SFR_OFFSET__=0x%x", avr_current_arch->sfr_offset); +#ifdef WITH_AVRLIBC + cpp_define (pfile, "__WITH_AVRLIBC__"); +#endif /* WITH_AVRLIBC */ + /* Define builtin macros so that the user can easily query if or if not non-generic address spaces (and which) are supported. This is only supported for C. For C++, a language extension is needed Index: gcc/config/avr/avrlibc.h =================================================================== --- gcc/config/avr/avrlibc.h (revision 0) +++ gcc/config/avr/avrlibc.h (revision 0) @@ -0,0 +1,31 @@ +/* Definitions of target machine for the GNU compiler collection + for Atmel AVR micro controller. + Copyright (C) 2012 + Free Software Foundation, Inc. + Contributed by Georg-Johann Lay (avr@gjlay.de) + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +<http://www.gnu.org/licenses/>. */ + +/* AVR-Libc implements functions from libgcc.a in libm.a, see PR54461. */ + +#undef LIBGCC_SPEC +#define LIBGCC_SPEC \ + "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lgcc -lm }}}}}" + +#undef LINK_GCC_C_SEQUENCE_SPEC +#define LINK_GCC_C_SEQUENCE_SPEC \ + "--start-group %G %L --end-group" Index: gcc/config.gcc =================================================================== --- gcc/config.gcc (revision 190873) +++ gcc/config.gcc (working copy) @@ -892,6 +892,10 @@ avr-*-rtems*) ;; avr-*-*) tm_file="elfos.h avr/elf.h avr/avr.h dbxelf.h avr/avr-stdint.h" + if test x${with_avrlibc} = xyes; then + tm_file="${tm_file} ${cpu_type}/avrlibc.h" + tm_defines="${tm_defines} WITH_AVRLIBC" + fi tmake_file="avr/t-avr avr/t-multilib" use_gcc_stdint=wrap extra_gcc_objs="driver-avr.o avr-devices.o"