https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92843
--- Comment #8 from jules at gcc dot gnu.org --- (In reply to Thomas Schwinge from comment #7) > We first need to establish a stable baseline, with test cases, and then (or, > as part of that) merge the several independent pieces of the big "OpenACC > reference count overhaul". OK -- I think I misunderstood your strategy here, which is to avoid committing that patch in one go. IMO (:-)) it's a clear improvement overall, particularly with the consistency checking. Test coverage could probably be better -- but, I'm sure many of the new tests introduced by the manual deep-copy patch would not operate correctly without the refcounting fixes underneath (and the two patches were all merged together to start with, of course). > > The existing code was rewritten for a > > reason -- that being, I hit various problems with it (albeit long since > > forgotten) that interfered with the manual deep-copy implementation. > > So you'll have to dig out your notes, and/or we'll have to figure out any > rationale again, now. Patches that change such fundamental things in > libgomp we cannot just commit on the basis that once they made sense to > somebody. They have to come with rationale, and test cases. That's holding the new code to a rather higher standard than some of the existing code! But, that's OK, I suppose. I will see if there are any notes I made at the time that might be helpful. > > We have > > refcount self-checking code for the overhauled code. > > ... which surely can be adapted. If you do that adaptation now, you might get a better idea of the state of the current refcounting implementation! Not sure if that'll be helpful. > And, per my understanding, this is only checking libgomp internal > consistency, but not the semantics exposed to users via OpenACC > directives/API calls etc., which in part you're changing of your patch > (without test cases). This, again, I've spent the best part of the past > weeks understanding, writing test cases for, filing GCC PRs, resolving them > one by one, independently, incrementally, comprehensibly. No, not just internal consistency. At least with the deep copy bits, there was user-visible breakage with the existing code (again, IIRC). > > We can address corner-case bug fixes as follow-ons once the main overhaul > > patch is committed. > > Further bug fixes, yes, but we have to make some reasonable effort to not > introduce new bugs with the big "OpenACC reference count overhaul" changes. That's what the testsuite is for, I guess -- the burden of proof for getting patches approved is lack of regressions in existing tests, generally. Yes there could be more, but this is a real fundamental thing that most of the existing OpenACC tests will touch in some way, so it's not *that* bad.
