[PATCH] RISC-V: fix build issue with gcc 4.9.x

[PATCH] RISC-V: fix build issue with gcc 4.9.x

[Romain Naour]

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

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Kito Cheng]

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
>

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Jeff Law]

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

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Kito Cheng]

> > 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

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Romain Naour]

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

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Kito Cheng]

> >>> 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
>

Re: [PATCH] RISC-V: fix build issue with gcc 4.9.x

[Andrew Pinski]

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
> >