This is an archive of the discontinued LLVM Phabricator instance.

OpenMP 5.0 metadirective
ClosedPublic

Authored by alokmishra.besu on Nov 23 2020, 12:56 AM.

Details

Summary

This patch supports OpenMP 5.0 metadirective features.
It is implemented keeping the OpenMP 5.1 features like dynamic user condition in mind.

A new function, getBestWhenMatchForContext, is defined in llvm/Frontend/OpenMP/OMPContext.h

Currently this function return the index of the when clause with the highest score from the ones applicable in the Context.
But this function is declared with an array which can be used in OpenMP 5.1 implementation to select all the valid when clauses which can be resolved in runtime. Currently this array is set to null by default and its implementation is left for future.

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 23 2020, 12:56 AM
alokmishra.besu requested review of this revision.Nov 23 2020, 12:56 AM
martong removed a subscriber: martong.Nov 23 2020, 1:47 AM

This looks close to an OpenMP 5.0 implementation. I left comments inlined.

We need tests that show how non-selected alternatives *do not* impact the program. As an example, a template instantiation inside of a non-selected alternative is not actually performed.

We also need test with ill-formed metadirectives.

clang/include/clang/Basic/DiagnosticSemaKinds.td
10782

no !. The default clause doesn't need to be in the end either.

clang/include/clang/Serialization/ASTBitCodes.h
1868–1869

Unrelated.

clang/lib/CodeGen/CGOpenMPRuntime.cpp
10590

Can you explain this, this seems odd to me.

clang/lib/Parse/ParseOpenMP.cpp
2336

Usually you would check the return value in case we later actually propagate errors while parsing the context selector.

2343

If we give up it should be an error, I think. If we issue a warning we just pretend the colon was there afterwards.

2354

We have balanced trackers for this that also deal with the fact that there might be a ) missing. This code will probably crash.

2366

Add a TODO that nullptr should be replaced as per the use in Sema::ActOnOpenMPCall.

2387

Use a BalancedDelimiterTracker.

2404

If you warn and continue above you need to check for : here again.

2410

What is this token? We have skipped this part before so we need to validate it is what we expect it to be.

2414

Should we not go back to the original code handling "directives" instead? This looks like it is copied here.

2467

Same as below, change the order. Also, the "skipping" part is always the same, put it in a helper function or lambda.

2474

Move the smaller case first and use an early exit. That will reduce the indention of the larger case by 1.

clang/test/OpenMP/metadirective_construct.cpp
12 ↗(On Diff #306974)

Since when does the construct trait work? I'm confused.

clang/test/OpenMP/metadirective_empty.cpp
2

no -fopenmp-targets please.

clang/test/OpenMP/metadirective_implementation.cpp
1 ↗(On Diff #306974)

Can we run this for all configurations of the metadirective so we can actually see it will pick the right one, not the first?

llvm/include/llvm/Frontend/OpenMP/OMPContext.h
197 ↗(On Diff #306974)

Leftover.

llvm/lib/Frontend/OpenMP/OMPContext.cpp
337 ↗(On Diff #306974)

This looks like a clone of getBestVariantMatchForContext with an extra unused argument.

400 ↗(On Diff #306974)

Leftover.

I have replied to the comments and will update the code accordingly. Some of the codes are intentionally left to be update in 5.1 implementation. Will add TODO comments there.
I will revisit the implementation of getBestWhenMatchForContext and also add more test cases over the weekend and submit new code by next week.

clang/include/clang/Basic/DiagnosticSemaKinds.td
10782

OK. Will update accordingly.

clang/include/clang/Serialization/ASTBitCodes.h
1868–1869

Only STMT_OMP_META_DIRECTIVE was added. Rest was formatted by git clang-format

clang/lib/CodeGen/CGOpenMPRuntime.cpp
10590

Good catch. This was an experimental code for 5.1. Got committed by mistake. Will update.

clang/lib/Parse/ParseOpenMP.cpp
2336

OK. Will update accordingly.

2343

OK. Will update accordingly.

2354

Come to think of it, in case of a missing ')' this code might end up in an infinite loop. Will update accordingly.

2366

Will do

2404

Will update accordingly.

2414

Unfortunately we cannot go to the original code handling since the original code handling assumes that the directive always ends with annot_pragma_openmp_end, while here it will always end with ')'.
In specification 5.0, since we are choosing only 1 directive, the body of the while block remains the same as the original code. Only the condition of the while block changes. In specification 5.1, we will need to generate code for dynamic handling and even the body will differ as we might need to generate AST node for multiple directives. It is best if we handle this code here for easier handling of 5.1 code, than in the original code space.
I will add a TODO comment here.

2467

OK

2474

OK

clang/test/OpenMP/metadirective_empty.cpp
2

OK

clang/test/OpenMP/metadirective_implementation.cpp
1 ↗(On Diff #306974)

Good catch. I will go through the implementation of getBestWhenMatchForContext to check that.

llvm/include/llvm/Frontend/OpenMP/OMPContext.h
197 ↗(On Diff #306974)

Will remove.

llvm/lib/Frontend/OpenMP/OMPContext.cpp
337 ↗(On Diff #306974)

I intended to keep it similar for 5.0 to be updated in 5.1 code. But anyways it seems to be giving wrong result. Will go through this again.

400 ↗(On Diff #306974)

Will remove.

Updated version of metadirective supporting OpenMP 5.0 features

ABataev added a subscriber: ABataev.Dec 3 2020, 8:41 AM
ABataev added inline comments.
clang/include/clang/AST/StmtOpenMP.h
610

I think, metadirective should be a kind of a container for different sub-directives. The problem is that that subdirectives could be completely different, they may capture different variables, using different capture kind (by value or by reference) etc.So, you need to generate each possible sub-directive independently and store them in the meta directive node. Otherwise you won't be able to generate the code correctly.

alokmishra.besu added inline comments.Dec 3 2020, 9:55 AM
clang/include/clang/AST/StmtOpenMP.h
610

In OpenMP 5.0, we do not need to generate every sub-directive. Rather we need to select one (or none) directive and replace metadirective with it. So this is not needed.
Yes with future specifications we will need to include a list of all valid directives which need to be resolved at runtime. That is when we will need to generate and store multiple sub-directives inside the OMPMetaDirective class.

ABataev added inline comments.Dec 3 2020, 10:09 AM
clang/include/clang/AST/StmtOpenMP.h
610

I think you still need to do it even for 5.0. I don't mean y need to emit the code for every sub-directive, but to generate them in the AST. Even in the example above, when(user={condition(N>10)} is used in the template and N is a template argument, the chosen directive can be selected on the instantiation and, thus, the original templated directive has to contain all possible sub-directives.

jdoerfert added inline comments.Dec 3 2020, 10:18 AM
clang/include/clang/Serialization/ASTBitCodes.h
1868–1869

Rest was formatted by git clang-format

Please don't format unrelated code.

clang/lib/Parse/ParseOpenMP.cpp
2414

Unfortunately we cannot go to the original code handling since the original code handling assumes that the directive always ends with annot_pragma_openmp_end, while here it will always end with ')'.

Let's add a flag to the original handling to make this possible then. Copying it is going to create more long term problems.

llvm/lib/Frontend/OpenMP/OMPContext.cpp
337 ↗(On Diff #306974)

Use the getBestVariantMatchForContext interface now and for the 5.1 semantics we introduce the OrderedMatch. That can also be used in the call site declare variant handling which currently calls getBestVariantMatchForContext multiple times.

alokmishra.besu added inline comments.Dec 3 2020, 3:12 PM
clang/include/clang/Serialization/ASTBitCodes.h
1868–1869

Removed.

clang/lib/Parse/ParseOpenMP.cpp
2414

Done.

This looks close to an OpenMP 5.0 implementation. I left comments inlined.

We need tests that show how non-selected alternatives *do not* impact the program. As an example, a template instantiation inside of a non-selected alternative is not actually performed.

We also need test with ill-formed metadirectives.

@jdoerfert I'm still trying to understand this thing regarding template instantiations. The spec says that a directive variant associated with a when clause can only affect the program if it is a dynamic replacement candidate. I had assumed this is referring to the runtime behavior of the program, and not (for instance) whether a compiler error is emitted due to a failing static assert.

Also, in order to determine what are the dynamic replacement candidates, and their order, all specified score expressions would need to be evaluated. Then, you'd need to evaluate the static context selectors, in decreasing order of their score, to find which one is the last dynamic replacement candidate. So I think template instantiations could be possible for those expressions, even if the corresponding variants aren't selected?

clang/include/clang/AST/StmtOpenMP.h
610

r

This looks close to an OpenMP 5.0 implementation. I left comments inlined.

We need tests that show how non-selected alternatives *do not* impact the program. As an example, a template instantiation inside of a non-selected alternative is not actually performed.

We also need test with ill-formed metadirectives.

@jdoerfert I'm still trying to understand this thing regarding template instantiations.

The problem is this patch can only resolve to a single directive during parsing. Take

template<bool b>
void foo() {
  #pragma omp metadirective when(user={condition(b)}) ...
}

which is not resolvable at parsing time but it is a valid OpenMP 5.0 use of the metadirective
that needs to resolve at compile time.

The spec says that a directive variant associated with a when clause can only affect the program if it is a dynamic replacement candidate. I had assumed this is referring to the runtime behavior of the program, and not (for instance) whether a compiler error is emitted due to a failing static assert.

I always assume(d) that static_assert is something "affecting the program". It doesn't matter though because if you instantiate all the clauses eagerly you change things for the runtime as well, e.g., you can cause different template instances to be selected later on depending on earlier instantiations. So it's not "just" static_assert.

Also, in order to determine what are the dynamic replacement candidates, and their order, all specified score expressions would need to be evaluated. Then, you'd need to evaluate the static context selectors, in decreasing order of their score, to find which one is the last dynamic replacement candidate. So I think template instantiations could be possible for those expressions, even if the corresponding variants aren't selected?

Yes, we will always evaluate selectors and scores. The key is that we won't evaluate the directive variant, which is neither a selector nor a score.

I hope this makes sense.

jdoerfert accepted this revision.Dec 22 2020, 9:30 AM

As discussed in the OpenMP in LLVM call on December 9th, we should go ahead with this patch and add support for the missing features, e.g., dependent selectors, afterwards. LGTM

This revision is now accepted and ready to land.Dec 22 2020, 9:30 AM

(Please make sure all clang and clang-tidy tests pass properly so we can merge the patch)

ABataev added inline comments.Dec 22 2020, 9:35 AM
clang/lib/Sema/TreeTransform.h
8519–8524

It would not work correctly, I would replace it with just error emission that instantiation is not supported yet.

alokmishra.besu marked an inline comment as done.

All clang test and clang-tidy pass. Ran clang-format on all code, except clang/include/clang/Serialization/ASTBitCodes.h which was formatting unrelated code as well.

Emitting error instead of instantiation in clang/lib/Sema/TreeTransform.h. Added a TODO for future.

Rebased with current branch to avoid patch failure

cchen added a subscriber: cchen.EditedJan 4 2021, 3:29 PM

@alokmishra.besu, I'm trying to play/learn with your patch so I downloaded the diff and apply it to the master branch, however, I'm getting error messages everywhere if llvm::omp::OMPD_metadirective appears:

llvm-project/clang/include/clang/AST/StmtOpenMP.h:384:43: error: no member named 'OMPD_metadirective' in namespace 'llvm::omp'
                               llvm::omp::OMPD_metadirective, SourceLocation(),
                               ~~~~~~~~~~~^

Do you know why this is happening? (I've also tried clean build it but still get the error msg) Thanks!

Edit: I've been successfully compiled it after changing a computer, thanks.

Are the test all passing?

I found two issues with this patch regarding the default clause:

  • The spec does not require a default clause. I get error: expected expression if I omit a default clause. The error is gone if I add default().
  • The spec does not allow an empty default() clause, but rather expects default(nothing)or omission of the default clause. This patch accepts the empty default clause.

I found two issues with this patch regarding the default clause:

  • The spec does not require a default clause. I get error: expected expression if I omit a default clause. The error is gone if I add default().
  • The spec does not allow an empty default() clause, but rather expects default(nothing)or omission of the default clause. This patch accepts the empty default clause.

There are various issues with this patch. We will fix them afterwards. The patch provides a lot of infrastructure we need. Please provide the problematic cases in a bug.

Are the test all passing?

Can we fix the failing tests so we can merge this?

Are the test all passing?

Can we fix the failing tests so we can merge this?

I will work on it this week. Will update soon.

I've rebase this patch with the latest git code.
All test cases pass.
I've also applied this patch to a new git clone. It applies and builds successfully.

There are no test cases anymore, as far as I can tell.

There are no test cases anymore, as far as I can tell.

Updated patch.

jdoerfert accepted this revision.Feb 3 2021, 1:49 PM

Asssuming this passes the tests I'll merge it as is. There are unresolved review requests, e.g., custom implementation instead of balanced parenthesis trackers, the template support, etc.
but we decided we can make faster progress in tree once this is merged.

The problem is this patch can only resolve to a single directive during parsing. Take

template<bool b>
void foo() {
  #pragma omp metadirective when(user={condition(b)}) ...
}

which is not resolvable at parsing time but it is a valid OpenMP 5.0 use of the metadirective
that needs to resolve at compile time.

It's unfortunate that this pattern is not supported by this patch. How difficult would it be to provide this support, or would the implementation be not different to dynamic conditions?
For this templated pattern, the optimizer would throw away the branch.

cchen added a comment.EditedSep 2 2021, 11:03 AM

I'm guessing the tests were not pass on buildbot but passed on the author's side is due to the assertion was disabled on the author's side.
Here is the patch for avoiding all the assertion errors and I'm able to get all the metadirective tests passed (and no regression for the existing omp tests) with this change:

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 5ac6dd0fb55a..3313efe524f3 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2277,8 +2277,10 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
 ///
 StmtResult
 Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
-  assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!");
   static bool ReadDirectiveWithinMetadirective = false;
+  if (!ReadDirectiveWithinMetadirective)
+    assert(Tok.isOneOf(tok::annot_pragma_openmp, tok::annot_attr_openmp) &&
+           "Not an OpenMP directive!");
   ParsingOpenMPDirectiveRAII DirScope(*this);
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
   SmallVector<OMPClause *, 5> Clauses;
@@ -2288,7 +2290,7 @@ Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
   unsigned ScopeFlags = Scope::FnScope | Scope::DeclScope |
                         Scope::CompoundStmtScope | Scope::OpenMPDirectiveScope;
   if (!ReadDirectiveWithinMetadirective)
-      ConsumeAnnotationToken();
+    ConsumeAnnotationToken();
   SourceLocation Loc = Tok.getLocation(), EndLoc;
   OpenMPDirectiveKind DKind = parseOpenMPDirectiveKind(*this);
   if (ReadDirectiveWithinMetadirective && DKind == OMPD_unknown) {
@@ -2331,6 +2333,7 @@ Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
         parseOMPContextSelectors(Loc, TI);
         if (TI.Sets.size() == 0) {
           Diag(Tok, diag::err_omp_expected_context_selector) << "when clause";
+          TPA.Commit();
           return Directive;
         }

@@ -2339,6 +2342,7 @@ Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
           ConsumeAnyToken();
         else {
           Diag(Tok, diag::err_omp_expected_colon) << "when clause";
+          TPA.Commit();
           return Directive;
         }
       }
@@ -2352,6 +2356,8 @@ Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
         if (Tok.is(tok::annot_pragma_openmp_end)) {
           Diag(Tok, diag::err_omp_expected_punc)
               << getOpenMPClauseName(CKind) << 0;
+          TPA.Commit();
           return Directive;
         }
         ConsumeAnyToken();
@@ -2627,7 +2633,7 @@ Parser::ParseOpenMPDeclarativeOrExecutableDirective(ParsedStmtContext StmtCtx) {
       // ends with a ')'.
       if (ReadDirectiveWithinMetadirective && Tok.is(tok::r_paren)) {
         while (Tok.isNot(tok::annot_pragma_openmp_end))
-          ConsumeToken();
+          ConsumeAnyToken();
         break;
       }
       bool HasImplicitClause = false;
cchen added a comment.Sep 8 2021, 1:07 PM

@alokmishra.besu do you mind if I push the patch for solving those assertions with rebase? The patch does not change the logic in your program and I've made sure that the tests could pass in debug mode.

@alokmishra.besu do you mind if I push the patch for solving those assertions with rebase? The patch does not change the logic in your program and I've made sure that the tests could pass in debug mode.

I think your patch is valid. Thanks for it. Please go ahead.

cchen updated this revision to Diff 372032.EditedSep 10 2021, 4:36 PM

Rebase and avoid assert errors

cchen updated this revision to Diff 372034.Sep 10 2021, 4:38 PM

Remove redundant file

cchen updated this revision to Diff 372038.Sep 10 2021, 4:52 PM

Remove debug info and spaces

cchen updated this revision to Diff 373109.Sep 16 2021, 4:58 PM

Fix ast errors and add some tiny fixes

cchen updated this revision to Diff 373125.Sep 16 2021, 7:55 PM

Rebase and fix warning

cchen updated this revision to Diff 373139.Sep 16 2021, 9:16 PM

Fix flang issue

cchen updated this revision to Diff 373277.Sep 17 2021, 10:32 AM

Fix tests for Windows

ABataev added inline comments.Sep 17 2021, 10:41 AM
clang/include/clang/AST/StmtOpenMP.h
5412
  1. Make it private, please
  2. Var names must start from capital letter stmt->S
clang/lib/Parse/ParseOpenMP.cpp
2389

Please, fix var names.

cchen added inline comments.Sep 17 2021, 12:11 PM
clang/include/clang/AST/StmtOpenMP.h
5412

I'm going to fix it. By the way, do you know how to fix those failing Windows tests? I tried to fix it by adding ifndef HEADER to test files and also replace var names with regex, but those Windows tests still fail. Thanks

ABataev added inline comments.Sep 17 2021, 12:15 PM
clang/include/clang/AST/StmtOpenMP.h
5412

No idea. Need a bit of context (I mean, would be good to see the test log etc.).

cchen added inline comments.Sep 17 2021, 12:21 PM
clang/include/clang/AST/StmtOpenMP.h
5412

I think the run line might be so general, I update it and see if that helps.

ABataev added inline comments.Sep 17 2021, 12:24 PM
clang/include/clang/AST/StmtOpenMP.h
5412

Just provide target triple in all test runs commands for linux and make a separate test for windows (if required), it should fix the issue with the testing.

cchen updated this revision to Diff 373312.Sep 17 2021, 12:41 PM

Fix tests and coding style

This revision was automatically updated to reflect the committed changes.
cchen added a comment.Sep 17 2021, 2:32 PM

I reapplied the patch since I forgot to add author's name in the commit

phosek added a subscriber: phosek.Sep 17 2021, 10:19 PM

We started seeing a test failure in Clang :: OpenMP/metadirective_device_kind_codegen.c on our macOS bot after this change landed (we aren't seeing it on other systems):

Script:
--
: 'RUN: at line 1';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 -internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/14.0.0/include -nostdsysteminc -verify -fopenmp -x c -std=c99 -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c -o - | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c
: 'RUN: at line 2';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 -internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/14.0.0/include -nostdsysteminc -verify -fopenmp -x c -triple x86_64-unknown-linux -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c -o - | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c
: 'RUN: at line 3';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 -internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/14.0.0/include -nostdsysteminc -verify -fopenmp -x c -triple aarch64-unknown-linux -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c -o - | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c
: 'RUN: at line 4';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 -internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/14.0.0/include -nostdsysteminc -verify -fopenmp -x c -triple ppc64le-unknown-linux -emit-llvm /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c -o - | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c
--
Exit Code: 1

Command Output (stderr):
--
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c:39:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: define {{.+}} void @foo()
                ^
<stdin>:1:1: note: scanning from here
; ModuleID = '/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c'
^
<stdin>:13:1: note: possible intended match here
define void @foo() #0 {
^

Input file: <stdin>
Check file: /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            1: ; ModuleID = '/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c' 
label:39'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
            2: source_filename = "/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/metadirective_device_kind_codegen.c" 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            3: target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            4: target triple = "x86_64-apple-darwin19.6.0" 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            5:  
label:39'0     ~
            6: %struct.ident_t = type { i32, i32, i32, i32, i8* } 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            7:  
label:39'0     ~
            8: @0 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            9: @1 = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @0, i32 0, i32 0) }, align 8 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           10: @2 = private unnamed_addr constant %struct.ident_t { i32 0, i32 514, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @0, i32 0, i32 0) }, align 8 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           11:  
label:39'0     ~
           12: ; Function Attrs: noinline nounwind optnone 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           13: define void @foo() #0 { 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~
label:39'1     ?                        possible intended match
           14: entry: 
label:39'0     ~~~~~~~
           15:  %0 = call i32 @__kmpc_global_thread_num(%struct.ident_t* @1) 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           16:  call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @.omp_outlined. to void (i32*, i32*, ...)*)) 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           17:  call void @__kmpc_push_num_threads(%struct.ident_t* @1, i32 %0, i32 4) 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           18:  call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @1, i32 0, void (i32*, i32*, ...)* bitcast (void (i32*, i32*)* @.omp_outlined..1 to void (i32*, i32*, ...)*)) 
label:39'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            .
            .
            .
>>>>>>

--

Would it be possible to take a look and either quickly address the issue or revert the change?

thakis added a subscriber: thakis.Sep 18 2021, 6:31 AM

(Reverted for now since mac bots have been broken for 16 hours now.)

Also, when this lands again, please ensure that all the tests don't accidentally dump build artifacts into the source directory. Specifically, the following test was missing -o -: clang/test/OpenMP/metadirective_messages.cpp

Looks like this was committed again, breaking the SystemZ build bots once again:
https://lab.llvm.org/buildbot/#/builders/94/builds/5661

cchen added a comment.Sep 20 2021, 9:55 AM

Looks like this was committed again, breaking the SystemZ build bots once again:
https://lab.llvm.org/buildbot/#/builders/94/builds/5661

I'm going to fix this issue now.

The SystemZ issue is due to the fact that we assumed that device(cpu) should be evaluated to true and device(gpu) should be evaluated to false in the test so the test should be fixed by specifying the triple. (https://github.com/llvm/llvm-project/commit/3679d2001c87f37101e7f20c646b21e97d8a0867)

The SystemZ issue is due to the fact that we assumed that device(cpu) should be evaluated to true and device(gpu) should be evaluated to false in the test so the test should be fixed by specifying the triple. (https://github.com/llvm/llvm-project/commit/3679d2001c87f37101e7f20c646b21e97d8a0867)

Thanks, it looks like this fixed the problem.