-
Notifications
You must be signed in to change notification settings - Fork 953
commit c5fd96ebb vs. gcc15 vs. ppcle64 in Fedora #4964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Ah, looks like we only have CI for gcc-13 or older. And gcc-14 was failing when it was set to gcc-13 as the newest supported. Should probably look into why that was failing. |
On Wed, Mar 26, 2025 at 03:15:53PM -0700, KrystalDelusion wrote:
(the latter most likely correlated with gcc version 15).
Ah, looks like we only have CI for gcc-13 or older. And gcc-14 was failing when
it was set to gcc-13 as the newest supported. Should probably look into why
that was failing.
The weird thing is that (for now) it only fails on
(gcc15 AND arch==ppcle64). I.e., it passes all tests
(even with gcc15) on x86_64.
On one hand, ppcle64 is the "tail wagging the dog" in
that I don't know anyone who might really care about
the intersection of yosys and that arch... :)
OTOH, just because it fails to fail on x86_64, doesn't
mean there isn't something subtly wrong and ppcle64 is
just an "early warning"...
|
Are you able to retrieve the
|
Are you able to test with latest main? We've had a few issues come through from the Debian folks that we've fixed for them, and I wonder if something in there might solve this too. |
I'm also going to try building the latest main, but that's going to take a few hours of ppcle64-on-x86 emulation... |
Oh, |
I had grabbed So yeah, breakage started happening some short time after the v051 release. I also tried latest As before, it builds fine on x86_64, but fails a (different this time) test on ppcle64. Here's the build log: b9131853f.build.log and also the log of the failing test: t_mem1.log EDIT: it's not quite failing the test, as much as the test leads it to coredump on ppcle64... |
Gotcha, thanks.
That's... odd. Having written it out both before and after the If there was anything between 0.51 and 0.51+85 that could've broken support on an obscure platform, I would've guessed it was #4834 which refactored io handling, rather than #4818 which you originally picked out. Are you able to test e44d1d4? That EDIT: As I was saying, that should include the complete macc_v2 PR and we can see if it's the same thing failing or not. Alternatively, if you still have access to the compiled b913185 in ppcle64 can you try run |
Dang misclick... |
The
suggests a failure in hash lib, but that also hasn't changed since before 0.51. |
attempting to build e44d1d4 on ppcle64 fails with the same error as
Here's a(nother?) copy of t_mem1.log |
That actually appears to be a different error, signal 11/segfault instead of signal6/abort... But both in the same place. Something is going very wrong there. Rather than running the full test suite, are you able to build Yosys (or reuse a previous build you still have it) and run this script:
I'm not sure if it will work, but could you also try to call it with |
Here's the output of Also, with This is with ppcle64 yosys build from commit e44d1d4. Thanks for looking into it, much appreciated! |
That's perfect. Running locally I get
compared to your
Which tells us that the crash is after
Especially with one of your earlier crashes giving the message
Which would most likely be in the The problem with that of course is that the Are you able to change |
I've just added an extra out-of-tree patch to the rpm build script (i.e., spec file) to do that, and it's currently going through the motions. I'll run the script again when it's done building. We'll also know whether the test(s) affected are still failing or not. Stay tuned... :) |
with the patch applied:
|
I've added |
turns out, "mock" uses user-mode qemu, which apparently does not support ptrace... :( |
Here it is: gdb.log I had to cut'n'paste from the terminal, as LMK if there's anything else I should try to collect. Thanks! EDIT: and here's the |
Okay so it's faulting in More specifically, it happens while looking up the representative And there it is, just tested with ASAN
Local reproduction*! *maybe, ASAN is giving the error during the |
Reverting #4892 seems to fix the ASAN error. @widlarizer was there a reason that #4892 rebuilds the indices at the end of the loop, as opposed to moving the @gsomlo can you test with this patch applied: diff --git a/passes/memory/memory_libmap.cc b/passes/memory/memory_libmap.cc
index b0d0498ea..b62375e7e 100644
--- a/passes/memory/memory_libmap.cc
+++ b/passes/memory/memory_libmap.cc
@@ -2232,10 +2232,10 @@ struct MemoryLibMapPass : public Pass {
if (module->has_processes_warn())
continue;
- MapWorker worker(module);
auto mems = Mem::get_selected_memories(module);
for (auto &mem : mems)
{
+ MapWorker worker(module);
MemMapping map(worker, mem, lib, opts);
int idx = -1;
int best = map.logic_cost;
@@ -2258,8 +2258,6 @@ struct MemoryLibMapPass : public Pass {
log("using FF mapping for memory %s.%s\n", log_id(module->name), log_id(mem.memid));
} else {
map.emit(map.cfgs[idx]);
- // Rebuild indices after modifying module
- worker = MapWorker(module);
}
}
} |
Built on top of new 0.52 tag with the patch applied. Build succeeds, all checks pass, no segfault on either ppcle64 or x86_64. That seems to fix the problem, thanks again for looking into it! |
I'm sure that one person with a powerpc digital synthesis workstation appreciates our hard work :3 |
On Tue, Apr 15, 2025 at 01:54:07AM -0700, Emil J wrote:
I'm sure that one person with a powerpc digital synthesis workstation
appreciates our hard work :3
Heh, I'm quite comfortable assuming that one person does not even
exist, so all the hard work shall remain unappreciated... :P
jk, UB is bad regardless of observed occurrences.
With my Fedora packager hat on, I prefer to look at the mandatory
"alternative" architectures on which packages must also be built and
tested as a slightly annoying (but likely beneficial in hindsight)
"early warning system" for just this kind of thing :)
Thanks for the report
Thank you all for the fix, and for looking after the project overall!
|
Version
commit c5fd96e
On which OS did this happen?
Linux
Reproduction Steps
I maintain yosys in Fedora. With each new release, the source RPM is built for a set of different architectures, including x86_64, aarch64, etc., including ppc64le.
While working on version 51, I noticed that
make test
fails, only on ppcle64, and only on fedora versions >= 42(the latter most likely correlated with gcc version 15).
After a bisect, I found the first commit where this happens to be c5fd96e ("macc_v2: Start new cell"), authored by @povik
Attached is the full ppcle64 fedora mock build log illustrating that problem. c5fd96ebb0c9067927bd4df381af92783bb4b547.build.log.txt
Expected Behavior
make test
succeeds on all architectures, including ppcle64Actual Behavior
make test
fails on ppcle64, gcc15 (fedora 42 and later)The text was updated successfully, but these errors were encountered: