RISC-V: fix build issue with gcc 4.9.x
Checks
Commit Message
GCC should still build with GCC 4.8.3 or newer [1]
using C++03 by default. But a recent change in
RISC-V port introduced a C++11 feature "std::log2" [2].
Use log2 from the C header, without the namespace [3].
[1] https://gcc.gnu.org/install/prerequisites.html
[2] https://gcc.gnu.org/git/?p=gcc.git;a=patch;h=7caa1ae5e451e780fbc4746a54e3f19d4f4304dc
[3] https://stackoverflow.com/questions/26733413/error-log2-is-not-a-member-of-std
Fixes:
https://gitlab.com/buildroot.org/toolchains-builder/-/jobs/4202276589
gcc/ChangeLog:
* config/riscv/genrvv-type-indexer.cc: Use log2 from the C header, without
the namespace.
Signed-off-by: Romain Naour <romain.naour@gmail.com>
---
gcc/config/riscv/genrvv-type-indexer.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
Hi Romain:
Pushed to trunk, thanks for catching that, that's definitely should
use log2 no matter C++03 or C++11,
but I think GCC allows the usage of C++11 according to
https://gcc.gnu.org/install/prerequisites.html :P
On Tue, May 2, 2023 at 8:22 PM Romain Naour via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> GCC should still build with GCC 4.8.3 or newer [1]
> using C++03 by default. But a recent change in
> RISC-V port introduced a C++11 feature "std::log2" [2].
>
> Use log2 from the C header, without the namespace [3].
>
> [1] https://gcc.gnu.org/install/prerequisites.html
> [2] https://gcc.gnu.org/git/?p=gcc.git;a=patch;h=7caa1ae5e451e780fbc4746a54e3f19d4f4304dc
> [3] https://stackoverflow.com/questions/26733413/error-log2-is-not-a-member-of-std
>
> Fixes:
> https://gitlab.com/buildroot.org/toolchains-builder/-/jobs/4202276589
>
> gcc/ChangeLog:
> * config/riscv/genrvv-type-indexer.cc: Use log2 from the C header, without
> the namespace.
>
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> ---
> gcc/config/riscv/genrvv-type-indexer.cc | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/riscv/genrvv-type-indexer.cc b/gcc/config/riscv/genrvv-type-indexer.cc
> index e677b55290c..eebe382d1c3 100644
> --- a/gcc/config/riscv/genrvv-type-indexer.cc
> +++ b/gcc/config/riscv/genrvv-type-indexer.cc
> @@ -115,9 +115,9 @@ same_ratio_eew_type (unsigned sew, int lmul_log2, unsigned eew, bool unsigned_p,
> if (sew == eew)
> elmul_log2 = lmul_log2;
> else if (sew > eew)
> - elmul_log2 = lmul_log2 - std::log2 (sew / eew);
> + elmul_log2 = lmul_log2 - log2 (sew / eew);
> else /* sew < eew */
> - elmul_log2 = lmul_log2 + std::log2 (eew / sew);
> + elmul_log2 = lmul_log2 + log2 (eew / sew);
>
> if (float_p)
> return floattype (eew, elmul_log2);
> --
> 2.34.3
>
On 5/2/23 08:31, Kito Cheng via Gcc-patches wrote:
> Hi Romain:
>
> Pushed to trunk, thanks for catching that, that's definitely should
> use log2 no matter C++03 or C++11,
> but I think GCC allows the usage of C++11 according to
> https://gcc.gnu.org/install/prerequisites.html :P
Yes, we should be able to use C++11. I'd like to get that to C++17 at
some point, but I think the biggest problem is the desire to support
bootstrapping on something like centos7/rhel7.
jeff
> > Pushed to trunk, thanks for catching that, that's definitely should
> > use log2 no matter C++03 or C++11,
> > but I think GCC allows the usage of C++11 according to
> > https://gcc.gnu.org/install/prerequisites.html :P
> Yes, we should be able to use C++11. I'd like to get that to C++17 at
> some point, but I think the biggest problem is the desire to support
> bootstrapping on something like centos7/rhel7.
At least we have auto and range based for loop, I am satisfied with
that enough.
>
> jeff
Hi Kito,
Le 02/05/2023 à 17:51, Kito Cheng a écrit :
>>> Pushed to trunk, thanks for catching that, that's definitely should
>>> use log2 no matter C++03 or C++11,
>>> but I think GCC allows the usage of C++11 according to
>>> https://gcc.gnu.org/install/prerequisites.html :P
>> Yes, we should be able to use C++11. I'd like to get that to C++17 at
>> some point, but I think the biggest problem is the desire to support
>> bootstrapping on something like centos7/rhel7.
>
> At least we have auto and range based for loop, I am satisfied with
> that enough.
Indeed, gcc 4.9 already support C++11 but for some reason std::log2 fail with
it. Probably because gcc 4.9 is the last gcc release with C++03 used by default.
I wasn't able to reproduce the build issue (without my patch) with gcc 10 or 11.
I'm fine with new prerequisites for the next gcc release, I checked the release
note about that. I noticed that all toolchains I've built with gcc 13.1 for
other cpu target where successful with the same setup (docker image).
Thanks!
Best regards,
Romain
>
>
>>
>> jeff
> >>> Pushed to trunk, thanks for catching that, that's definitely should
> >>> use log2 no matter C++03 or C++11,
> >>> but I think GCC allows the usage of C++11 according to
> >>> https://gcc.gnu.org/install/prerequisites.html :P
> >> Yes, we should be able to use C++11. I'd like to get that to C++17 at
> >> some point, but I think the biggest problem is the desire to support
> >> bootstrapping on something like centos7/rhel7.
> >
> > At least we have auto and range based for loop, I am satisfied with
> > that enough.
>
> Indeed, gcc 4.9 already support C++11 but for some reason std::log2 fail with
> it. Probably because gcc 4.9 is the last gcc release with C++03 used by default.
> I wasn't able to reproduce the build issue (without my patch) with gcc 10 or 11.
Anyway I think your fix is reasonable since we include math.h not cmath.h here,
so use without std:: is rightway, but don't know why it is build-able
with newer GCC,
I guess that might be included by some other header indirectly.
> I'm fine with new prerequisites for the next gcc release, I checked the release
> note about that. I noticed that all toolchains I've built with gcc 13.1 for
> other cpu target where successful with the same setup (docker image).
>
> Thanks!
>
> Best regards,
> Romain
>
> >
> >
> >>
> >> jeff
>
On Tue, May 2, 2023 at 5:38 PM Kito Cheng via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > >>> Pushed to trunk, thanks for catching that, that's definitely should
> > >>> use log2 no matter C++03 or C++11,
> > >>> but I think GCC allows the usage of C++11 according to
> > >>> https://gcc.gnu.org/install/prerequisites.html :P
> > >> Yes, we should be able to use C++11. I'd like to get that to C++17 at
> > >> some point, but I think the biggest problem is the desire to support
> > >> bootstrapping on something like centos7/rhel7.
> > >
> > > At least we have auto and range based for loop, I am satisfied with
> > > that enough.
> >
> > Indeed, gcc 4.9 already support C++11 but for some reason std::log2 fail with
> > it. Probably because gcc 4.9 is the last gcc release with C++03 used by default.
> > I wasn't able to reproduce the build issue (without my patch) with gcc 10 or 11.
>
> Anyway I think your fix is reasonable since we include math.h not cmath.h here,
> so use without std:: is rightway, but don't know why it is build-able
> with newer GCC,
> I guess that might be included by some other header indirectly.
No libstdc++ was specifically fixed in GCC 6 to do the correct thing,
see PR 60401 and PR 14608 (r6-6392-g96e19adabc80146648825).
Thanks,
Andrew Pinski
>
> > I'm fine with new prerequisites for the next gcc release, I checked the release
> > note about that. I noticed that all toolchains I've built with gcc 13.1 for
> > other cpu target where successful with the same setup (docker image).
> >
> > Thanks!
> >
> > Best regards,
> > Romain
> >
> > >
> > >
> > >>
> > >> jeff
> >
@@ -115,9 +115,9 @@ same_ratio_eew_type (unsigned sew, int lmul_log2, unsigned eew, bool unsigned_p,
if (sew == eew)
elmul_log2 = lmul_log2;
else if (sew > eew)
- elmul_log2 = lmul_log2 - std::log2 (sew / eew);
+ elmul_log2 = lmul_log2 - log2 (sew / eew);
else /* sew < eew */
- elmul_log2 = lmul_log2 + std::log2 (eew / sew);
+ elmul_log2 = lmul_log2 + log2 (eew / sew);
if (float_p)
return floattype (eew, elmul_log2);