[Openmp-commits] [PATCH] D153032: [OpenMP][OMPT] Change OMPT kind for OpenMP test lock functions

Jan André Reuter via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Mon Jun 26 01:41:46 PDT 2023


Thyre added a comment.

In D153032#4447949 <https://reviews.llvm.org/D153032#4447949>, @protze.joachim wrote:

> The whole stack looks good to me. My only concern is that with separate patches, the first two patches will lead to failing test pipelines.

I can see that, yeah. 
>From my perspective, it would make sense if D153028 <https://reviews.llvm.org/D153028> causes a failing pipeline since we are changing a test case which was incorrect before. The second patch (D153031 <https://reviews.llvm.org/D153031>) doesn't fix the issue, but is important to allow the test to pass in the third patch. It's unfortunate that this patch still causes the pipeline to fail.

I guess there are three options on how we can handle this:

1. Keep the patches as they are right now
2. Rebase the patches to include D153031 <https://reviews.llvm.org/D153031> in either the first or third patch
3. Squash all patches down to a single patch

I would tend to the first or second option. For the second option, I would include it in the bug fix itself to keep the test case a non-functional change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153032/new/

https://reviews.llvm.org/D153032



More information about the Openmp-commits mailing list