[llvm] [coro][pgo] Remove redundant coroutine test files (PR #89620)
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 22 10:36:08 PDT 2024
mtrofin wrote:
> > This was intentional, see the comment that introduced them ([#71262 (review)](https://github.com/llvm/llvm-project/pull/71262#pullrequestreview-1723261241))
>
> I saw that, but I don't agree with the outcome :-)
>
> > An advantage is that the tests cover 2 concerns, really: coro lowering, and PGO instrumentation. They can potentially diverge if needed.
>
> The original tests (e.g. llvm/test/Transforms/Coroutines/coro-split-musttail.ll) already cover both concerns nicely.
>
> If there's a need to diverge, new tests could be created when that need arises.
>
> > Other than needing to duplicate a fix in both places, is there any other shortcoming of the double presence?
>
> Needing to update two almost identical copies of 12 tests is a pretty big pain.
How about removing the PGO `RUN:` from the coro side and leaving these on the PGO side handle that? (still want to hear from @david-xl if he had any other concerns addressed by the duplication tho)
One reason I could think of, to keep the PGO side, is for code review auto-notification (i.e. because the pgo pr "team" is auto-added)
https://github.com/llvm/llvm-project/pull/89620
More information about the llvm-commits
mailing list