| Message ID | 6ecc7322-9335-4197-9508-34af9946c491@BAMAIL02.ba.imgtec.org |
|---|---|
| State | New |
| Headers | show |
On 05/01/16 14:43 -0800, Steve Ellcey wrote: >While trying to build GCC with uclibc instead of glibc I ran into a build >failure in libstdc++. uclibc doesn't seem to provide the isfinite function >like glibc does so that means that libstdc++ doesn't have std::isfinite. > >This in turn caused include/ext/random.tcc to not compile because it uses >std::isfinite. I can easily see how this might be considered a uclibc >bug but given that it is the only build problem I ran into I was hoping >we could fix it in libstdc++ by using __builtin_finite instead of Shouldn't that be __builtin_isfinite not __builtin_finite ? >std::isfinite. This fixes my uclibc build and also worked for glibc >with no regressions. > >OK to checkin? >Steve Ellcey >sellcey@imgtec.com > > >2016-01-05 Steve Ellcey <sellcey@imgtec.com> > > * include/ext/random.tcc: Use __builtin_finite instead of > std::isfinite. > > >diff --git a/libstdc++-v3/include/ext/random.tcc b/libstdc++-v3/include/ext/random.tcc >index a9c5a2b..3467823 100644 >--- a/libstdc++-v3/include/ext/random.tcc >+++ b/libstdc++-v3/include/ext/random.tcc >@@ -1570,7 +1570,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > return __t; }); > __norm = std::sqrt(__sum); > } >- while (__norm == _RealType(0) || ! std::isfinite(__norm)); >+ while (__norm == _RealType(0) || ! __builtin_finite(__norm)); > > std::transform(__ret.begin(), __ret.end(), __ret.begin(), > [__norm](_RealType __val){ return __val / __norm; });
On Wed, 2016-01-06 at 20:34 +0000, Jonathan Wakely wrote: > On 05/01/16 14:43 -0800, Steve Ellcey wrote: > >While trying to build GCC with uclibc instead of glibc I ran into a build > >failure in libstdc++. uclibc doesn't seem to provide the isfinite function > >like glibc does so that means that libstdc++ doesn't have std::isfinite. > > > >This in turn caused include/ext/random.tcc to not compile because it uses > >std::isfinite. I can easily see how this might be considered a uclibc > >bug but given that it is the only build problem I ran into I was hoping > >we could fix it in libstdc++ by using __builtin_finite instead of > > Shouldn't that be __builtin_isfinite not __builtin_finite ? You are right. For some reason I thought that isfinite was implemented as __builtin_finite and that there was no __builtin_isfinite. I am not sure why I thought that but it does not seem to be the case. I will update my patch, retest, and resubmit. Steve Ellcey sellcey@imgtec.com
On 06/01/16 12:45 -0800, Steve Ellcey wrote: >On Wed, 2016-01-06 at 20:34 +0000, Jonathan Wakely wrote: >> On 05/01/16 14:43 -0800, Steve Ellcey wrote: >> >While trying to build GCC with uclibc instead of glibc I ran into a build >> >failure in libstdc++. uclibc doesn't seem to provide the isfinite function >> >like glibc does so that means that libstdc++ doesn't have std::isfinite. >> > >> >This in turn caused include/ext/random.tcc to not compile because it uses >> >std::isfinite. I can easily see how this might be considered a uclibc >> >bug but given that it is the only build problem I ran into I was hoping >> >we could fix it in libstdc++ by using __builtin_finite instead of >> >> Shouldn't that be __builtin_isfinite not __builtin_finite ? > >You are right. For some reason I thought that isfinite was implemented >as __builtin_finite and that there was no __builtin_isfinite. I am not >sure why I thought that but it does not seem to be the case. I will >update my patch, retest, and resubmit. Thanks. Regarding the substance of the patch, the usual approach would be to gate the relevant parts of <random> (or even the whole thing) on the presence of std::isinfinite, but that would presumably disable the <random> functionality completely for uclibc. If using the builtin works reliably then I'm OK with that. However, I would expect that for un-optimized code the builtin will just emit a call to isfinite() in the C library, and so if uclibc doesn't define that at all then that will result in a linker error. If uclibc does define that function then we should figure out why libstdc++ fails to see it, and see if we can fix that, so that std::isfinite() gets declared and the existing code in <random> starts working. i.e. I'd like to be sure there isn't some deeper underlying problem that should be addressed, rather than just working around it.
On Wed, 6 Jan 2016, Jonathan Wakely wrote: > On 06/01/16 12:45 -0800, Steve Ellcey wrote: >> On Wed, 2016-01-06 at 20:34 +0000, Jonathan Wakely wrote: >>> On 05/01/16 14:43 -0800, Steve Ellcey wrote: >>> >While trying to build GCC with uclibc instead of glibc I ran into a build >>> >failure in libstdc++. uclibc doesn't seem to provide the isfinite >>> function >>> >like glibc does so that means that libstdc++ doesn't have std::isfinite. >>> > >>> >This in turn caused include/ext/random.tcc to not compile because it uses >>> >std::isfinite. I can easily see how this might be considered a uclibc >>> >bug but given that it is the only build problem I ran into I was hoping >>> >we could fix it in libstdc++ by using __builtin_finite instead of >>> >>> Shouldn't that be __builtin_isfinite not __builtin_finite ? >> >> You are right. For some reason I thought that isfinite was implemented >> as __builtin_finite and that there was no __builtin_isfinite. I am not >> sure why I thought that but it does not seem to be the case. I will >> update my patch, retest, and resubmit. > > Thanks. > > Regarding the substance of the patch, the usual approach would be to > gate the relevant parts of <random> (or even the whole thing) on the > presence of std::isinfinite, but that would presumably disable the > <random> functionality completely for uclibc. If using the builtin > works reliably then I'm OK with that. > > However, I would expect that for un-optimized code the builtin will > just emit a call to isfinite() in the C library, and so if uclibc > doesn't define that at all then that will result in a linker error. I thought that was half the reason you asked for isfinite (the other half being that __builtin_isfinite is generic while __builtin_finite (and the [fl] variants) is not). If you look at builtins.def, isfinite is declared with DEF_GCC_BUILTIN (no library fallback), as opposed to finite with DEF_EXT_LIB_BUILTIN.
On 07/01/16 07:28 +0100, Marc Glisse wrote: >On Wed, 6 Jan 2016, Jonathan Wakely wrote: > >>On 06/01/16 12:45 -0800, Steve Ellcey wrote: >>>On Wed, 2016-01-06 at 20:34 +0000, Jonathan Wakely wrote: >>>>On 05/01/16 14:43 -0800, Steve Ellcey wrote: >>>>>While trying to build GCC with uclibc instead of glibc I ran into a build >>>>>failure in libstdc++. uclibc doesn't seem to provide the >>>>isfinite function >>>>>like glibc does so that means that libstdc++ doesn't have std::isfinite. >>>>> >>>>>This in turn caused include/ext/random.tcc to not compile because it uses >>>>>std::isfinite. I can easily see how this might be considered a uclibc >>>>>bug but given that it is the only build problem I ran into I was hoping >>>>>we could fix it in libstdc++ by using __builtin_finite instead of >>>> >>>>Shouldn't that be __builtin_isfinite not __builtin_finite ? >>> >>>You are right. For some reason I thought that isfinite was implemented >>>as __builtin_finite and that there was no __builtin_isfinite. I am not >>>sure why I thought that but it does not seem to be the case. I will >>>update my patch, retest, and resubmit. >> >>Thanks. >> >>Regarding the substance of the patch, the usual approach would be to >>gate the relevant parts of <random> (or even the whole thing) on the >>presence of std::isinfinite, but that would presumably disable the >><random> functionality completely for uclibc. If using the builtin >>works reliably then I'm OK with that. >> >>However, I would expect that for un-optimized code the builtin will >>just emit a call to isfinite() in the C library, and so if uclibc >>doesn't define that at all then that will result in a linker error. > >I thought that was half the reason you asked for isfinite (the other >half being that __builtin_isfinite is generic while __builtin_finite >(and the [fl] variants) is not). If you look at builtins.def, isfinite >is declared with DEF_GCC_BUILTIN (no library fallback), as opposed to >finite with DEF_EXT_LIB_BUILTIN. You give me too much credit :-) I was only concerned with the generic property, but if it doesn't require a library fallback then __builtin_isfinite should work fine, and we don't need to worry about investigating whether something is wrong with the autoconf checks.
On January 7, 2016 12:23:49 PM GMT+01:00, Jonathan Wakely <jwakely@redhat.com> wrote: >On 07/01/16 07:28 +0100, Marc Glisse wrote: >>On Wed, 6 Jan 2016, Jonathan Wakely wrote: >> >>>On 06/01/16 12:45 -0800, Steve Ellcey wrote: >>>>On Wed, 2016-01-06 at 20:34 +0000, Jonathan Wakely wrote: >>>>>On 05/01/16 14:43 -0800, Steve Ellcey wrote: >>>>>>While trying to build GCC with uclibc instead of glibc I ran into >a build >>>>>>failure in libstdc++. uclibc doesn't seem to provide the >>>>>isfinite function uClibc provides an infinite macro. Make sure to enable C99 math though. OK? TIA, >>>>>>like glibc does so that means that libstdc++ doesn't have >std::isfinite. >>>>>> >>>>>>This in turn caused include/ext/random.tcc to not compile because >it uses >>>>>>std::isfinite. I can easily see how this might be considered a >uclibc >>>>>>bug but given that it is the only build problem I ran into I was >hoping >>>>>>we could fix it in libstdc++ by using __builtin_finite instead of >>>>> >>>>>Shouldn't that be __builtin_isfinite not __builtin_finite ? >>>> >>>>You are right. For some reason I thought that isfinite was >implemented >>>>as __builtin_finite and that there was no __builtin_isfinite. I am >not >>>>sure why I thought that but it does not seem to be the case. I will >>>>update my patch, retest, and resubmit. >>> >>>Thanks. >>> >>>Regarding the substance of the patch, the usual approach would be to >>>gate the relevant parts of <random> (or even the whole thing) on the >>>presence of std::isinfinite, but that would presumably disable the >>><random> functionality completely for uclibc. If using the builtin >>>works reliably then I'm OK with that. >>> >>>However, I would expect that for un-optimized code the builtin will >>>just emit a call to isfinite() in the C library, and so if uclibc >>>doesn't define that at all then that will result in a linker error. >> >>I thought that was half the reason you asked for isfinite (the other >>half being that __builtin_isfinite is generic while __builtin_finite >>(and the [fl] variants) is not). If you look at builtins.def, isfinite > >>is declared with DEF_GCC_BUILTIN (no library fallback), as opposed to >>finite with DEF_EXT_LIB_BUILTIN. > >You give me too much credit :-) > >I was only concerned with the generic property, but if it doesn't >require a library fallback then __builtin_isfinite should work fine, >and we don't need to worry about investigating whether something is >wrong with the autoconf checks.
diff --git a/libstdc++-v3/include/ext/random.tcc b/libstdc++-v3/include/ext/random.tcc index a9c5a2b..3467823 100644 --- a/libstdc++-v3/include/ext/random.tcc +++ b/libstdc++-v3/include/ext/random.tcc @@ -1570,7 +1570,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return __t; }); __norm = std::sqrt(__sum); } - while (__norm == _RealType(0) || ! std::isfinite(__norm)); + while (__norm == _RealType(0) || ! __builtin_finite(__norm)); std::transform(__ret.begin(), __ret.end(), __ret.begin(), [__norm](_RealType __val){ return __val / __norm; });