Message ID | 00e701da424c$c8bb0b60$5a312220$@nextmovesoftware.com |
---|---|
State | New |
Headers | show |
Series | [libatomic] Fix testsuite regressions on ARM [raspberry pi]. | expand |
On 08/01/2024 16:07, Roger Sayle wrote: > > Bootstrapping GCC on arm-linux-gnueabihf with --with-arch=armv6 currently > has a large number of FAILs in libatomic (regressions since last time I > attempted this). The failure mode is related to IFUNC handling with the > file tas_8_2_.o containing an unresolved reference to the function > libat_test_and_set_1_i2. > > Bearing in mind I've no idea what's going on, the following one line > change, to build tas_1_2_.o when building tas_8_2_.o, resolves the problem > for me and restores the libatomic testsuite to 44 expected passes and 5 > unsupported tests [from 22 unexpected failures and 22 unresolved testcases]. > > If this looks like the correct fix, I'm not confident with rebuilding > Makefile.in with correct version of automake, so I'd very much appreciate > it if someone/the reviewer/mainainer could please check this in for me. > Thanks in advance. > > > 2024-01-08 Roger Sayle <roger@nextmovesoftware.com> > > libatomic/ChangeLog > * Makefile.am: Build tas_1_2_.o on ARCH_ARM_LINUX > * Makefile.in: Regenerate. > > > Roger > -- > Hi Roger, I don't really understand all this make foo :( so I'm not sure if this is the right fix either. If this is, as you say, a regression, have you been able to track down when it first started to occur? That might also help me to understand what changed to cause this. Perhaps we should have a PR for this, to make tracking the fixes easier. R.
Hi Richard, As you've recommended, this issue has now been filed in bugzilla as PR other/113336. As explained in the new PR, libatomic's testsuite used to pass on armv6 (raspberry pi) in previous GCC releases, but the code was incorrect/non-synchronous; this was reported as PR target/107567 and PR target/109166. Now that those issues have been fixed, we now see that there's a missing dependency in libatomic that's required to implement this functionality correctly. I'm more convinced that my fix is correct, but it's perhaps a little disappointing that libatomic doesn't have a (multi-threaded) run-time test to search for race conditions, and confirm its implementations are correctly serializing. Please let me know what you think. Best regards, Roger -- > -----Original Message----- > From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> > Sent: 10 January 2024 15:34 > To: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org > Subject: Re: [libatomic PATCH] Fix testsuite regressions on ARM [raspberry pi]. > > > > On 08/01/2024 16:07, Roger Sayle wrote: > > > > Bootstrapping GCC on arm-linux-gnueabihf with --with-arch=armv6 > > currently has a large number of FAILs in libatomic (regressions since > > last time I attempted this). The failure mode is related to IFUNC > > handling with the file tas_8_2_.o containing an unresolved reference > > to the function libat_test_and_set_1_i2. > > > > Bearing in mind I've no idea what's going on, the following one line > > change, to build tas_1_2_.o when building tas_8_2_.o, resolves the > > problem for me and restores the libatomic testsuite to 44 expected > > passes and 5 unsupported tests [from 22 unexpected failures and 22 unresolved > testcases]. > > > > If this looks like the correct fix, I'm not confident with rebuilding > > Makefile.in with correct version of automake, so I'd very much > > appreciate it if someone/the reviewer/mainainer could please check this in for > me. > > Thanks in advance. > > > > > > 2024-01-08 Roger Sayle <roger@nextmovesoftware.com> > > > > libatomic/ChangeLog > > * Makefile.am: Build tas_1_2_.o on ARCH_ARM_LINUX > > * Makefile.in: Regenerate. > > > > > > Roger > > -- > > > > Hi Roger, > > I don't really understand all this make foo :( so I'm not sure if this is the right fix > either. If this is, as you say, a regression, have you been able to track down when > it first started to occur? That might also help me to understand what changed to > cause this. > > Perhaps we should have a PR for this, to make tracking the fixes easier. > > R.
On 1/11/24 15:55, Roger Sayle wrote: > > Hi Richard, > As you've recommended, this issue has now been filed in bugzilla > as PR other/113336. As explained in the new PR, libatomic's testsuite > used to pass on armv6 (raspberry pi) in previous GCC releases, but > the code was incorrect/non-synchronous; this was reported as > PR target/107567 and PR target/109166. Now that those issues > have been fixed, we now see that there's a missing dependency in > libatomic that's required to implement this functionality correctly. > > I'm more convinced that my fix is correct, but it's perhaps a little > disappointing that libatomic doesn't have a (multi-threaded) run-time > test to search for race conditions, and confirm its implementations > are correctly serializing. > > Please let me know what you think. > Best regards, > Roger > -- I do think that if the regression is caused by HAVE_ATOMIC_TAS now being detected as false due to a bugfix elsewhere as you kindly pointed out, then the fix perhaps ought to change the compile-time behavior for TAS alone. As I point out in Bugzilla, we can get away with replacing the proposed libatomic_la_LIBADD += $(addsuffix _1_2_.lo,$(SIZEOBJS)) with libatomic_la_LIBADD += tas_1_2_.lo so that we generate the missing `libat_test_and_set_1_i2' specifically. I've not manage to detect the need for any other *_1_i2 thus far and this alone appears sufficient to fix all observed regressions. Happy to investigate further, but my initial findings seem to be that this may be a better fix. Let me know if you disagree ;). Regards, Victor >> -----Original Message----- >> From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> >> Sent: 10 January 2024 15:34 >> To: Roger Sayle <roger@nextmovesoftware.com>; gcc-patches@gcc.gnu.org >> Subject: Re: [libatomic PATCH] Fix testsuite regressions on ARM [raspberry pi]. >> >> >> >> On 08/01/2024 16:07, Roger Sayle wrote: >>> >>> Bootstrapping GCC on arm-linux-gnueabihf with --with-arch=armv6 >>> currently has a large number of FAILs in libatomic (regressions since >>> last time I attempted this). The failure mode is related to IFUNC >>> handling with the file tas_8_2_.o containing an unresolved reference >>> to the function libat_test_and_set_1_i2. >>> >>> Bearing in mind I've no idea what's going on, the following one line >>> change, to build tas_1_2_.o when building tas_8_2_.o, resolves the >>> problem for me and restores the libatomic testsuite to 44 expected >>> passes and 5 unsupported tests [from 22 unexpected failures and 22 unresolved >> testcases]. >>> >>> If this looks like the correct fix, I'm not confident with rebuilding >>> Makefile.in with correct version of automake, so I'd very much >>> appreciate it if someone/the reviewer/mainainer could please check this in for >> me. >>> Thanks in advance. >>> >>> >>> 2024-01-08 Roger Sayle <roger@nextmovesoftware.com> >>> >>> libatomic/ChangeLog >>> * Makefile.am: Build tas_1_2_.o on ARCH_ARM_LINUX >>> * Makefile.in: Regenerate. >>> >>> >>> Roger >>> -- >>> >> >> Hi Roger, >> >> I don't really understand all this make foo :( so I'm not sure if this is the right fix >> either. If this is, as you say, a regression, have you been able to track down when >> it first started to occur? That might also help me to understand what changed to >> cause this. >> >> Perhaps we should have a PR for this, to make tracking the fixes easier. >> >> R. >
diff --git a/libatomic/Makefile.am b/libatomic/Makefile.am index cfad90124f9..e0988a18c9a 100644 --- a/libatomic/Makefile.am +++ b/libatomic/Makefile.am @@ -139,6 +139,7 @@ if ARCH_ARM_LINUX IFUNC_OPTIONS = -march=armv7-a+fp -DHAVE_KERNEL64 libatomic_la_LIBADD += $(foreach s,$(SIZES),$(addsuffix _$(s)_1_.lo,$(SIZEOBJS))) libatomic_la_LIBADD += $(addsuffix _8_2_.lo,$(SIZEOBJS)) +libatomic_la_LIBADD += $(addsuffix _1_2_.lo,$(SIZEOBJS)) endif if ARCH_I386 IFUNC_OPTIONS = -march=i586