| Message ID | DBBPR83MB0613D9E7A152C1830A079CD4F8922@DBBPR83MB0613.EURPRD83.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | SMALL code model fixes, optimization fixes, LTO and minimal C++ enablement | expand |
Hi Evgeny, > On 2 Sep 2024, at 15:03, Evgeny Karpov <Evgeny.Karpov@microsoft.com> wrote: > > External email: Use caution opening links or attachments > > > The patch resolves compilation issues for the C++ language. Previous > patch series contributed to C++ as well, however, C++ could not be > tested until we got a C++ compiler and could build at least a "Hello > World" C++ program, and in reality, more than that. > > gcc/ChangeLog: > > * config.gcc: Add missing dependencies. > > libstdc++-v3/ChangeLog: > > * src/c++17/fast_float/fast_float.h (defined): Adjust a condition > for AArch64. libstdc++ is reviewed on its own list (CC’ed here) so I’d suggest splitting the libstdc++-v3 hunk into its own patch and submitting it separately there for review. Thanks, Kyrill > --- > gcc/config.gcc | 1 + > libstdc++-v3/src/c++17/fast_float/fast_float.h | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/gcc/config.gcc b/gcc/config.gcc > index a36dd1bcbc6..e1117c273f0 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -1283,6 +1283,7 @@ aarch64-*-mingw*) > extra_options="${extra_options} mingw/cygming.opt mingw/mingw.opt" > extra_objs="${extra_objs} winnt.o winnt-dll.o" > c_target_objs="${c_target_objs} msformat-c.o" > + cxx_target_objs="${cxx_target_objs} msformat-c.o" > d_target_objs="${d_target_objs} winnt-d.o" > tmake_file="${tmake_file} mingw/t-cygming" > case ${enable_threads} in > diff --git a/libstdc++-v3/src/c++17/fast_float/fast_float.h b/libstdc++-v3/src/c++17/fast_float/fast_float.h > index 7551c4f89ef..dc61de7b199 100644 > --- a/libstdc++-v3/src/c++17/fast_float/fast_float.h > +++ b/libstdc++-v3/src/c++17/fast_float/fast_float.h > @@ -275,7 +275,8 @@ fastfloat_really_inline value128 full_multiplication(uint64_t a, > // But MinGW on ARM64 doesn't have native support for 64-bit multiplications > answer.high = __umulh(a, b); > answer.low = a * b; > -#elif defined(FASTFLOAT_32BIT) || (defined(_WIN64) && !defined(__clang__)) > +#elif defined (FASTFLOAT_32BIT) || (defined (_WIN64) && !defined (__clang__) \ > + && !defined (_M_ARM64)) > answer.low = _umul128(a, b, &answer.high); // _umul128 not available on ARM64 > #elif defined(FASTFLOAT_64BIT) > __uint128_t r = ((__uint128_t)a) * b; > -- > 2.34.1 >
On Mon, 2 Sept 2024, 14:15 Kyrylo Tkachov, <ktkachov@nvidia.com> wrote: > Hi Evgeny, > > > On 2 Sep 2024, at 15:03, Evgeny Karpov <Evgeny.Karpov@microsoft.com> > wrote: > > > > External email: Use caution opening links or attachments > > > > > > The patch resolves compilation issues for the C++ language. Previous > > patch series contributed to C++ as well, however, C++ could not be > > tested until we got a C++ compiler and could build at least a "Hello > > World" C++ program, and in reality, more than that. > > > > gcc/ChangeLog: > > > > * config.gcc: Add missing dependencies. > > > > libstdc++-v3/ChangeLog: > > > > * src/c++17/fast_float/fast_float.h (defined): Adjust a condition > > for AArch64. > > libstdc++ is reviewed on its own list (CC’ed here) so I’d suggest > splitting the libstdc++-v3 hunk into its own patch and submitting it > separately there for review. > fast_float is an external project there we import. Has this fix been sent upstream? Thanks, > Kyrill > > > --- > > gcc/config.gcc | 1 + > > libstdc++-v3/src/c++17/fast_float/fast_float.h | 3 ++- > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/config.gcc b/gcc/config.gcc > > index a36dd1bcbc6..e1117c273f0 100644 > > --- a/gcc/config.gcc > > +++ b/gcc/config.gcc > > @@ -1283,6 +1283,7 @@ aarch64-*-mingw*) > > extra_options="${extra_options} mingw/cygming.opt mingw/mingw.opt" > > extra_objs="${extra_objs} winnt.o winnt-dll.o" > > c_target_objs="${c_target_objs} msformat-c.o" > > + cxx_target_objs="${cxx_target_objs} msformat-c.o" > > d_target_objs="${d_target_objs} winnt-d.o" > > tmake_file="${tmake_file} mingw/t-cygming" > > case ${enable_threads} in > > diff --git a/libstdc++-v3/src/c++17/fast_float/fast_float.h > b/libstdc++-v3/src/c++17/fast_float/fast_float.h > > index 7551c4f89ef..dc61de7b199 100644 > > --- a/libstdc++-v3/src/c++17/fast_float/fast_float.h > > +++ b/libstdc++-v3/src/c++17/fast_float/fast_float.h > > @@ -275,7 +275,8 @@ fastfloat_really_inline value128 > full_multiplication(uint64_t a, > > // But MinGW on ARM64 doesn't have native support for 64-bit > multiplications > > answer.high = __umulh(a, b); > > answer.low = a * b; > > -#elif defined(FASTFLOAT_32BIT) || (defined(_WIN64) && > !defined(__clang__)) > > +#elif defined (FASTFLOAT_32BIT) || (defined (_WIN64) && !defined > (__clang__) \ > > + && !defined (_M_ARM64)) > > answer.low = _umul128(a, b, &answer.high); // _umul128 not available > on ARM64 > > #elif defined(FASTFLOAT_64BIT) > > __uint128_t r = ((__uint128_t)a) * b; > > -- > > 2.34.1 > > > >
On Mon, 2 Sept 2024, 14:15 Kyrylo Tkachov, <ktkachov@nvidia.com> wrote: > Hi Evgeny, > > > On 2 Sep 2024, at 15:03, Evgeny Karpov <Evgeny.Karpov@microsoft.com> > wrote: > > > > External email: Use caution opening links or attachments > > > > > > The patch resolves compilation issues for the C++ language. Previous > > patch series contributed to C++ as well, however, C++ could not be > > tested until we got a C++ compiler and could build at least a "Hello > > World" C++ program, and in reality, more than that. > > > > gcc/ChangeLog: > > > > * config.gcc: Add missing dependencies. > > > > libstdc++-v3/ChangeLog: > > > > * src/c++17/fast_float/fast_float.h (defined): Adjust a condition > > for AArch64. > It looks like this changelog was created by the mklog script, but it needs fixing. This is not changing "defined". It looks like it's changing "full_multiplication" so that's what should be named in the changelog. > libstdc++ is reviewed on its own list (CC’ed here) so I’d suggest > splitting the libstdc++-v3 hunk into its own patch and submitting it > separately there for review. > Thanks, > Kyrill > > > --- > > gcc/config.gcc | 1 + > > libstdc++-v3/src/c++17/fast_float/fast_float.h | 3 ++- > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/config.gcc b/gcc/config.gcc > > index a36dd1bcbc6..e1117c273f0 100644 > > --- a/gcc/config.gcc > > +++ b/gcc/config.gcc > > @@ -1283,6 +1283,7 @@ aarch64-*-mingw*) > > extra_options="${extra_options} mingw/cygming.opt mingw/mingw.opt" > > extra_objs="${extra_objs} winnt.o winnt-dll.o" > > c_target_objs="${c_target_objs} msformat-c.o" > > + cxx_target_objs="${cxx_target_objs} msformat-c.o" > > d_target_objs="${d_target_objs} winnt-d.o" > > tmake_file="${tmake_file} mingw/t-cygming" > > case ${enable_threads} in > > diff --git a/libstdc++-v3/src/c++17/fast_float/fast_float.h > b/libstdc++-v3/src/c++17/fast_float/fast_float.h > > index 7551c4f89ef..dc61de7b199 100644 > > --- a/libstdc++-v3/src/c++17/fast_float/fast_float.h > > +++ b/libstdc++-v3/src/c++17/fast_float/fast_float.h > > @@ -275,7 +275,8 @@ fastfloat_really_inline value128 > full_multiplication(uint64_t a, > > // But MinGW on ARM64 doesn't have native support for 64-bit > multiplications > > answer.high = __umulh(a, b); > > answer.low = a * b; > > -#elif defined(FASTFLOAT_32BIT) || (defined(_WIN64) && > !defined(__clang__)) > > +#elif defined (FASTFLOAT_32BIT) || (defined (_WIN64) && !defined > (__clang__) \ > > + && !defined (_M_ARM64)) > > answer.low = _umul128(a, b, &answer.high); // _umul128 not available > on ARM64 > > #elif defined(FASTFLOAT_64BIT) > > __uint128_t r = ((__uint128_t)a) * b; > > -- > > 2.34.1 > > > >
Monday, September 2, 2024 3:15 PM Kyrylo Tkachov <ktkachov@nvidia.com> wrote: >> libstdc++-v3/ChangeLog: >> >> * src/c++17/fast_float/fast_float.h (defined): Adjust a condition >> for AArch64. > > libstdc++ is reviewed on its own list (CC’ed here) so I’d suggest splitting > the libstdc++-v3 hunk into its own patch and submitting it separately there for review. Monday, September 2, 2024 3:45 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > fast_float is an external project there we import. Has this fix been sent upstream? > It looks like this changelog was created by the mklog script, but it needs > fixing. This is not changing "defined". It looks like it's changing > "full_multiplication" so that's what should be named in the changelog. Thank you, Kyrylo and Jonathan, for clarifying the process for upstreaming changes to fast_float. The change has been upstreamed to the fast_float and libstdc++ patch has been prepared. https://gcc.gnu.org/pipermail/libstdc++/2024-September/059472.html
diff --git a/gcc/config.gcc b/gcc/config.gcc index a36dd1bcbc6..e1117c273f0 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -1283,6 +1283,7 @@ aarch64-*-mingw*) extra_options="${extra_options} mingw/cygming.opt mingw/mingw.opt" extra_objs="${extra_objs} winnt.o winnt-dll.o" c_target_objs="${c_target_objs} msformat-c.o" + cxx_target_objs="${cxx_target_objs} msformat-c.o" d_target_objs="${d_target_objs} winnt-d.o" tmake_file="${tmake_file} mingw/t-cygming" case ${enable_threads} in diff --git a/libstdc++-v3/src/c++17/fast_float/fast_float.h b/libstdc++-v3/src/c++17/fast_float/fast_float.h index 7551c4f89ef..dc61de7b199 100644 --- a/libstdc++-v3/src/c++17/fast_float/fast_float.h +++ b/libstdc++-v3/src/c++17/fast_float/fast_float.h @@ -275,7 +275,8 @@ fastfloat_really_inline value128 full_multiplication(uint64_t a, // But MinGW on ARM64 doesn't have native support for 64-bit multiplications answer.high = __umulh(a, b); answer.low = a * b; -#elif defined(FASTFLOAT_32BIT) || (defined(_WIN64) && !defined(__clang__)) +#elif defined (FASTFLOAT_32BIT) || (defined (_WIN64) && !defined (__clang__) \ + && !defined (_M_ARM64)) answer.low = _umul128(a, b, &answer.high); // _umul128 not available on ARM64 #elif defined(FASTFLOAT_64BIT) __uint128_t r = ((__uint128_t)a) * b;