| Message ID | f3381264-616a-6c76-3357-7dec1f60696d@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | libffi: fix handling of homogeneous float128 structs [PR109447] | expand |
On 5/4/23 2:29 PM, Peter Bergner wrote: > I'd like to pull in Dan's upstream libffi commit into trunk to fix a > wrong code bug/testsuite failure on powerpc64le-linux with long double > defaulting to ieee128. This passed bootstrap and regtesting with no > regressions. Ok for trunk? > > This bug is also on the GCC 12 and GCC 11 release branches. Ok there too > assuming testing is clean? I can wait to push the gcc12 backport until > after the release. Oops, and of course, this needs to be backported to GCC 13 as well. Peter
On Thu, May 04, 2023 at 02:29:34PM -0500, Peter Bergner wrote: > I'd like to pull in Dan's upstream libffi commit into trunk to fix a > wrong code bug/testsuite failure on powerpc64le-linux with long double > defaulting to ieee128. This passed bootstrap and regtesting with no > regressions. Ok for trunk? > > This bug is also on the GCC 12 and GCC 11 release branches. Ok there too > assuming testing is clean? I can wait to push the gcc12 backport until > after the release. > > Peter > > > If there is a homogeneous struct with float128 members, they should be > copied to vector register save area. The current code incorrectly copies > only the value of the first member, not increasing the pointer with each > iteration. Fix this. > > Merged from upstream libffi commit: 464b4b66e3cf3b5489e730c1466ee1bf825560e0 > > 2023-05-03 Dan Horák <dan@danny.cz> > > libffi/ > PR libffi/109447 > * src/powerpc/ffi_linux64.c (ffi_prep_args64): Update arg.f128 pointer. Ok for 14/13.2/12.4 (i.e. after 12.3 is out)/11.4 > diff --git a/libffi/src/powerpc/ffi_linux64.c b/libffi/src/powerpc/ffi_linux64.c > index 4d50878e402..3454dacd3d6 100644 > --- a/libffi/src/powerpc/ffi_linux64.c > +++ b/libffi/src/powerpc/ffi_linux64.c > @@ -680,7 +680,7 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack) > { > if (vecarg_count < NUM_VEC_ARG_REGISTERS64 > && i < nfixedargs) > - memcpy (vec_base.f128++, arg.f128, sizeof (float128)); > + memcpy (vec_base.f128++, arg.f128++, sizeof (float128)); > else > memcpy (next_arg.f128, arg.f128++, sizeof (float128)); > if (++next_arg.f128 == gpr_end.f128) Jakub
On 5/5/23 4:42 PM, Jakub Jelinek wrote: > On Thu, May 04, 2023 at 02:29:34PM -0500, Peter Bergner wrote: >> Merged from upstream libffi commit: 464b4b66e3cf3b5489e730c1466ee1bf825560e0 >> >> 2023-05-03 Dan Horák <dan@danny.cz> >> >> libffi/ >> PR libffi/109447 >> * src/powerpc/ffi_linux64.c (ffi_prep_args64): Update arg.f128 pointer. > > Ok for 14/13.2/12.4 (i.e. after 12.3 is out)/11.4 Thanks, I've pushed the GCC trunk and GCC 13 commits and now that GCC 12.3 is released, I have pushed the GCC 12 backport too. I have yet to push to GCC 11 yet, due to bootstrap is broken when building GCC 11 on our Fedora 36 system, so I cannot test there yet. It also seems GCC 11 is missing some IEEE128 changes from upstream libffi that GCC 12 and later have, so it might not even be appropriate, but I'll wait for bootstrap to be restored before making any decisions. The problem seem to be that the system ld.gold which is used to link libgo.so is dying with an undefined runtime symbol: /usr/bin/ld.gold: /home/bergner/gcc/build/gcc-fsf-11-baselib-regtest/powerpc64le-linux/libstdc++-v3/src/.libs/libstdc++.so.6: version `GLIBCXX_3.4.30' not found (required by /usr/bin/ld.gold) collect2: error: ld returned 1 exit status Running the link command by hand or via a make in powerpc64le-linux/libgo, the link succeeds. It's only when I type make in the top level build dir where I see this error. It's almost as if the top level build machinery adds a LD_LIBRARY_PATH=... forcing the system ld.gold (which was built with a gcc12 based compiler) to pick up the build's gcc11 libstdc++ which doesn't have that symbol, rather than the gcc12 system libstdc++. Has anyone seen anything like that before? Peter
On Mai 09 2023, Peter Bergner via Gcc-patches wrote: > It's almost as if the top level build machinery > adds a LD_LIBRARY_PATH=... See how the toplevel Makefile sets LD_LIBRARY_PATH (via RPATH_ENVVAR) if gcc-bootstrap is set.
On 5/9/23 3:50 PM, Andreas Schwab wrote: > On Mai 09 2023, Peter Bergner via Gcc-patches wrote: > >> It's almost as if the top level build machinery >> adds a LD_LIBRARY_PATH=... > > See how the toplevel Makefile sets LD_LIBRARY_PATH (via RPATH_ENVVAR) if > gcc-bootstrap is set. I'm sorry to be dense, but can you point to the specific line? In my $GCC_BUILD/Makefile, the only mention of LD_LIBRARY_PATH is: RPATH_ENVVAR = LD_LIBRARY_PATH ...so that isn't setting LD_LIBRARY_PATH, but using it. Peter
On Mai 09 2023, Peter Bergner via Gcc-patches wrote: > On 5/9/23 3:50 PM, Andreas Schwab wrote: >> On Mai 09 2023, Peter Bergner via Gcc-patches wrote: >> >>> It's almost as if the top level build machinery >>> adds a LD_LIBRARY_PATH=... >> >> See how the toplevel Makefile sets LD_LIBRARY_PATH (via RPATH_ENVVAR) if >> gcc-bootstrap is set. > > I'm sorry to be dense, but can you point to the specific line? In my > $GCC_BUILD/Makefile, the only mention of LD_LIBRARY_PATH is: > > RPATH_ENVVAR = LD_LIBRARY_PATH > > ...so that isn't setting LD_LIBRARY_PATH, but using it. Have you considered searching for uses of RPATH_ENVVAR? $ grep RPATH_ENVVAR Makefile.in RPATH_ENVVAR = @RPATH_ENVVAR@ # On targets where RPATH_ENVVAR is PATH, a subdirectory of the GCC build path $(RPATH_ENVVAR)=`echo "$(TARGET_LIB_PATH)$$$(RPATH_ENVVAR)" | sed 's,::*,:,g;s,^:*,,;s,:*$$,,'`; export $(RPATH_ENVVAR); \ $(RPATH_ENVVAR)=`echo "$(HOST_LIB_PATH)$$$(RPATH_ENVVAR)" | sed 's,::*,:,g;s,^:*,,;s,:*$$,,'`; export $(RPATH_ENVVAR); $(RPATH_ENVVAR)=`echo "$(TARGET_LIB_PATH)$$$(RPATH_ENVVAR)" | sed 's,::*,:,g;s,^:*,,;s,:*$$,,'`; export $(RPATH_ENVVAR); \ $(RPATH_ENVVAR)=`echo "$(HOST_LIB_PATH)$$$(RPATH_ENVVAR)" | sed 's,::*,:,g;s,^:*,,;s,:*$$,,'`; export $(RPATH_ENVVAR); \ # This is the list of directories that may be needed in RPATH_ENVVAR # This is the list of directories that may be needed in RPATH_ENVVAR "RPATH_ENVVAR=$(RPATH_ENVVAR)" \
On 5/10/23 2:34 AM, Andreas Schwab wrote: > On Mai 09 2023, Peter Bergner via Gcc-patches wrote: >> I'm sorry to be dense, but can you point to the specific line? In my >> $GCC_BUILD/Makefile, the only mention of LD_LIBRARY_PATH is: >> >> RPATH_ENVVAR = LD_LIBRARY_PATH >> >> ...so that isn't setting LD_LIBRARY_PATH, but using it. > > Have you considered searching for uses of RPATH_ENVVAR? Ah, I misread that as RPATH_ENVVAR = $LD_LIBRARY_PATH and was expecting to see "export LD_LIBRARY_PATH=..." in the code. Thanks for the pointer! Peter
diff --git a/libffi/src/powerpc/ffi_linux64.c b/libffi/src/powerpc/ffi_linux64.c index 4d50878e402..3454dacd3d6 100644 --- a/libffi/src/powerpc/ffi_linux64.c +++ b/libffi/src/powerpc/ffi_linux64.c @@ -680,7 +680,7 @@ ffi_prep_args64 (extended_cif *ecif, unsigned long *const stack) { if (vecarg_count < NUM_VEC_ARG_REGISTERS64 && i < nfixedargs) - memcpy (vec_base.f128++, arg.f128, sizeof (float128)); + memcpy (vec_base.f128++, arg.f128++, sizeof (float128)); else memcpy (next_arg.f128, arg.f128++, sizeof (float128)); if (++next_arg.f128 == gpr_end.f128)