from:"Julian Brown via cfe\-commits"

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-13 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From c06ce375baf4a73818a225ec806fe143bee730fa Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 22 +
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7..513af9846aa7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2444,6 +2444,7 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
   case OMPD_target_teams_loop:
   case OMPD_parallel_loop:
   case OMPD_target_parallel_loop:
+  case OMPD_assume:
 Diag(Tok, diag::err_omp_unexpected_directive)
 << 1 << getOpenMPDirectiveName(DKind);
 break;
@@ -3023,6 +3024,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..731d839caef03 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..a38380ed4482a
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 0..33c1c6f7c51e7
-

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From d83105c32a48d73fe547523841a115d6863f7799 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH 1/2] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 22 +
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7..513af9846aa7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2444,6 +2444,7 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
   case OMPD_target_teams_loop:
   case OMPD_parallel_loop:
   case OMPD_target_parallel_loop:
+  case OMPD_assume:
 Diag(Tok, diag::err_omp_unexpected_directive)
 << 1 << getOpenMPDirectiveName(DKind);
 break;
@@ -3023,6 +3024,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..731d839caef03 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..a38380ed4482a
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 0..33c1c6f7c51

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From d83105c32a48d73fe547523841a115d6863f7799 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 22 +
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7..513af9846aa7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2444,6 +2444,7 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
   case OMPD_target_teams_loop:
   case OMPD_parallel_loop:
   case OMPD_target_parallel_loop:
+  case OMPD_assume:
 Diag(Tok, diag::err_omp_unexpected_directive)
 << 1 << getOpenMPDirectiveName(DKind);
 break;
@@ -3023,6 +3024,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..731d839caef03 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..a38380ed4482a
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 0..33c1c6f7c51e7
-

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-06-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created https://github.com/llvm/llvm-project/pull/96087

This patch adds a diagnostic which attempts to detect the case where the 
"collapse" clause is used with imperfectly-nested parallel loops, something 
like this:

  #pragma omp target
  #pragma omp parallel for collapse(2)
for (int i = 0; i < N; i++) {
  arr[i][i] = ...;
  for (int j = 0; j < N; j++) {
arr[i][j] = ...;
  }
}

This kind of nesting is permitted by OpenMP 5+.

At a glance, this appears fine: the outer loop iterations are independent, so 
can be executed in parallel, and the inner loop iterations are also independent 
and can be executed in parallel.

However, the "collapse" clause works by essentially moving the 
not-perfectly-nested statements into the innermost loop.  This is sometimes 
harmless but inefficient (the statement gets executed more times than a naive 
user might expect), but in this case the combined/collapsed loop iterations now 
have a data dependency between them:

  for (int ij = 0; ij < N*N; ij++) {
int i = ij / N, j = ij % N;
arr[i][i] = ...; // all of these...
arr[i][j] = ...; // ...would have to be executed before all of these
  }

...and that means the result is (silently!) incorrect.

Since this seems like an easy mistake to make, I was interested to find out if 
there was a feasible and reasonably-accurate way to try to diagnose it.  This 
is what I came up with.

Firstly, in Clang, memory load/store instructions emitted from statements in 
the "imperfect" parts of loop nests are annotated with a new annotation, 
"llvm.omp.loop.imperfection".  Then in LLVM proper, in the OpenMPOpt pass 
(because I couldn't find anywhere that looked more appropriate), memory 
load/store instructions in collapsed loops are partitioned into two groups, 
with or without the annotation.  Then if any of the first group may/must alias 
with any in the second group, a warning (actually a "remark") is emitted.

The remark is opt-in.  The user must compile with
"-Rpass-analysis=openmp-opt" to trigger it.  That seems appropriate, because 
the diagnostic potentially has a false-positive rate that is too high for a 
regular warning, but on the other hand users aren't likely to benefit from the 
true-positive warning unless they know to use the option. Comments welcome.

FWIW, I don't think there's a reasonable, safe way to collapse loops like this 
and maintain parallel semantics, but ICBW.

>From c2c199ddda26bad52c4a879a57837286dfb5c5fd Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 12 Jun 2024 13:58:22 -0500
Subject: [PATCH] [OpenMP] Diagnostic check for imperfect loop collapse

This patch adds a diagnostic which attempts to detect the case where the
"collapse" clause is used with imperfectly-nested parallel loops,
something like this:

  #pragma omp target
  #pragma omp parallel for collapse(2)
for (int i = 0; i < N; i++) {
  arr[i][i] = ...;
  for (int j = 0; j < N; j++) {
arr[i][j] = ...;
  }
}

This kind of nesting is permitted by OpenMP 5+.

At a glance, this appears fine: the outer loop iterations are
independent, so can be executed in parallel, and the inner loop
iterations are also independent and can be executed in parallel.

However, the "collapse" clause works by essentially moving the
not-perfectly-nested statements into the innermost loop.  This is
sometimes harmless but inefficient (the statement gets executed more times
than a naive user might expect), but in this case the combined/collapsed
loop iterations now have a data dependency between them:

  for (int ij = 0; ij < N*N; ij++) {
int i = ij / N, j = ij % N;
arr[i][i] = ...; // all of these...
arr[i][j] = ...; // ...would have to be executed before all of these
  }

...and that means the result is (silently!) incorrect.

Since this seems like an easy mistake to make, I was interested to find
out if there was a feasible and reasonably-accurate way to try to
diagnose it.  This is what I came up with.

Firstly, in Clang, memory load/store instructions emitted from
statements in the "imperfect" parts of loop nests are annotated with
a new annotation, "llvm.omp.loop.imperfection".  Then in LLVM proper,
in the OpenMPOpt pass (because I couldn't find anywhere that looked more
appropriate), memory load/store instructions in collapsed loops are
partitioned into two groups, with or without the annotation.  Then if
any of the first group may/must alias with any in the second group,
a warning (actually a "remark") is emitted.

The remark is opt-in.  The user must compile with
"-Rpass-analysis=openmp-opt" to trigger it.  That seems appropriate,
because the diagnostic potentially has a false-positive rate that is too
high for a regular warning, but on the other hand users aren't likely to
benefit from the true-positive warning unless they know to use the option.
Comments welcome.

FWIW, I don't think there's a reasonable, safe way to collapse loops
like this and maintain

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-06-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

Adding @jdoerfert

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

Ping?

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-06-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 edited https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-21 Thread Julian Brown via cfe-commits

https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 822249a1f45ce1341e71a9c99dec081d8e8d077f Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 12 Jun 2024 13:58:22 -0500
Subject: [PATCH] [OpenMP] Diagnostic check for imperfect loop collapse

This patch adds a diagnostic which attempts to detect the case where the
"collapse" clause is used with imperfectly-nested parallel loops,
something like this:

  #pragma omp target
  #pragma omp parallel for collapse(2)
for (int i = 0; i < N; i++) {
  arr[i][i] = ...;
  for (int j = 0; j < N; j++) {
arr[i][j] = ...;
  }
}

This kind of nesting is permitted by OpenMP 5+.

At a glance, this appears fine: the outer loop iterations are
independent, so can be executed in parallel, and the inner loop
iterations are also independent and can be executed in parallel.

However, the "collapse" clause works by essentially moving the
not-perfectly-nested statements into the innermost loop.  This is
sometimes harmless but inefficient (the statement gets executed more times
than a naive user might expect), but in this case the combined/collapsed
loop iterations now have a data dependency between them:

  for (int ij = 0; ij < N*N; ij++) {
int i = ij / N, j = ij % N;
arr[i][i] = ...; // all of these...
arr[i][j] = ...; // ...would have to be executed before all of these
  }

...and that means the result is (silently!) incorrect.

Since this seems like an easy mistake to make, I was interested to find
out if there was a feasible and reasonably-accurate way to try to
diagnose it.  This is what I came up with.

Firstly, in Clang, memory load/store instructions emitted from
statements in the "imperfect" parts of loop nests are annotated with
a new annotation, "llvm.omp.loop.imperfection".  Then in LLVM proper,
in the OpenMPOpt pass (because I couldn't find anywhere that looked more
appropriate), memory load/store instructions in collapsed loops are
partitioned into two groups, with or without the annotation.  Then if
any of the first group may/must alias with any in the second group,
a warning (actually a "remark") is emitted.

The remark is opt-in.  The user must compile with
"-Rpass-analysis=openmp-opt" to trigger it.  That seems appropriate,
because the diagnostic potentially has a false-positive rate that is too
high for a regular warning, but on the other hand users aren't likely to
benefit from the true-positive warning unless they know to use the option.
Comments welcome.

FWIW, I don't think there's a reasonable, safe way to collapse loops
like this and maintain parallel semantics, but ICBW.
---
 clang/lib/CodeGen/CGStmtOpenMP.cpp   |  7 ++-
 clang/lib/CodeGen/CodeGenFunction.cpp| 14 -
 clang/lib/CodeGen/CodeGenFunction.h  | 22 +++
 clang/test/OpenMP/for_collapse_imperfect.cpp | 65 
 llvm/lib/Transforms/IPO/OpenMPOpt.cpp| 56 +
 5 files changed, 162 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/OpenMP/for_collapse_imperfect.cpp

diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index f73d32de7c484..4a210bbea734c 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1898,7 +1898,12 @@ static void emitBody(CodeGenFunction &CGF, const Stmt 
*S, const Stmt *NextLoop,
   return;
 }
   }
-  CGF.EmitStmt(S);
+  if (SimplifiedS != NextLoop) {
+CodeGenFunction::OMPLoopImperfectionRAII OLI(CGF);
+CGF.EmitStmt(S);
+  } else {
+CGF.EmitStmt(S);
+  }
 }

 void CodeGenFunction::EmitOMPLoopBody(const OMPLoopDirective &D,
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 200c40da8bc43..dc945ce26b99c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -44,6 +44,8 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/xxhash.h"
 #include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"
@@ -2647,8 +2649,18 @@ void CGBuilderInserter::InsertHelper(
 llvm::Instruction *I, const llvm::Twine &Name,
 llvm::BasicBlock::iterator InsertPt) const {
   llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, InsertPt);
-  if (CGF)
+  if (CGF) {
 CGF->InsertHelper(I, Name, InsertPt);
+if (CGF->GetOMPLoopImperfection() &&
+I->mayReadOrWriteMemory()) {
+  llvm::LLVMContext &Ctx = CGF->getLLVMContext();
+  llvm::MDNode *Imp = llvm::MDNode::get(Ctx,
+llvm::ConstantAsMetadata::get(
+  llvm::ConstantInt::get(
+  llvm::Type::getInt1Ty(Ctx), 1)));
+  I->setMetadata("llvm.omp.loop.imperfection", Imp);
+}
+  }
 }

 // Emits an error if

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-06-21 Thread Julian Brown via cfe-commits

https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/96087

>From 822249a1f45ce1341e71a9c99dec081d8e8d077f Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 12 Jun 2024 13:58:22 -0500
Subject: [PATCH] [OpenMP] Diagnostic check for imperfect loop collapse

This patch adds a diagnostic which attempts to detect the case where the
"collapse" clause is used with imperfectly-nested parallel loops,
something like this:

  #pragma omp target
  #pragma omp parallel for collapse(2)
for (int i = 0; i < N; i++) {
  arr[i][i] = ...;
  for (int j = 0; j < N; j++) {
arr[i][j] = ...;
  }
}

This kind of nesting is permitted by OpenMP 5+.

At a glance, this appears fine: the outer loop iterations are
independent, so can be executed in parallel, and the inner loop
iterations are also independent and can be executed in parallel.

However, the "collapse" clause works by essentially moving the
not-perfectly-nested statements into the innermost loop.  This is
sometimes harmless but inefficient (the statement gets executed more times
than a naive user might expect), but in this case the combined/collapsed
loop iterations now have a data dependency between them:

  for (int ij = 0; ij < N*N; ij++) {
int i = ij / N, j = ij % N;
arr[i][i] = ...; // all of these...
arr[i][j] = ...; // ...would have to be executed before all of these
  }

...and that means the result is (silently!) incorrect.

Since this seems like an easy mistake to make, I was interested to find
out if there was a feasible and reasonably-accurate way to try to
diagnose it.  This is what I came up with.

Firstly, in Clang, memory load/store instructions emitted from
statements in the "imperfect" parts of loop nests are annotated with
a new annotation, "llvm.omp.loop.imperfection".  Then in LLVM proper,
in the OpenMPOpt pass (because I couldn't find anywhere that looked more
appropriate), memory load/store instructions in collapsed loops are
partitioned into two groups, with or without the annotation.  Then if
any of the first group may/must alias with any in the second group,
a warning (actually a "remark") is emitted.

The remark is opt-in.  The user must compile with
"-Rpass-analysis=openmp-opt" to trigger it.  That seems appropriate,
because the diagnostic potentially has a false-positive rate that is too
high for a regular warning, but on the other hand users aren't likely to
benefit from the true-positive warning unless they know to use the option.
Comments welcome.

FWIW, I don't think there's a reasonable, safe way to collapse loops
like this and maintain parallel semantics, but ICBW.
---
 clang/lib/CodeGen/CGStmtOpenMP.cpp   |  7 ++-
 clang/lib/CodeGen/CodeGenFunction.cpp| 14 -
 clang/lib/CodeGen/CodeGenFunction.h  | 22 +++
 clang/test/OpenMP/for_collapse_imperfect.cpp | 65 
 llvm/lib/Transforms/IPO/OpenMPOpt.cpp| 56 +
 5 files changed, 162 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/OpenMP/for_collapse_imperfect.cpp

diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index f73d32de7c484..4a210bbea734c 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1898,7 +1898,12 @@ static void emitBody(CodeGenFunction &CGF, const Stmt 
*S, const Stmt *NextLoop,
   return;
 }
   }
-  CGF.EmitStmt(S);
+  if (SimplifiedS != NextLoop) {
+CodeGenFunction::OMPLoopImperfectionRAII OLI(CGF);
+CGF.EmitStmt(S);
+  } else {
+CGF.EmitStmt(S);
+  }
 }

 void CodeGenFunction::EmitOMPLoopBody(const OMPLoopDirective &D,
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 200c40da8bc43..dc945ce26b99c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -44,6 +44,8 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Metadata.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/xxhash.h"
 #include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"
@@ -2647,8 +2649,18 @@ void CGBuilderInserter::InsertHelper(
 llvm::Instruction *I, const llvm::Twine &Name,
 llvm::BasicBlock::iterator InsertPt) const {
   llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, InsertPt);
-  if (CGF)
+  if (CGF) {
 CGF->InsertHelper(I, Name, InsertPt);
+if (CGF->GetOMPLoopImperfection() &&
+I->mayReadOrWriteMemory()) {
+  llvm::LLVMContext &Ctx = CGF->getLLVMContext();
+  llvm::MDNode *Imp = llvm::MDNode::get(Ctx,
+llvm::ConstantAsMetadata::get(
+  llvm::ConstantInt::get(
+  llvm::Type::getInt1Ty(Ctx), 1)));
+  I->setMetadata("llvm.omp.loop.imperfection", Imp);
+}
+  }
 }

 // Emits an error if

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-21 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From edbcd82b8a91766cea9e988e0f37acd685ff7d97 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 22 +
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7..513af9846aa7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2444,6 +2444,7 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
   case OMPD_target_teams_loop:
   case OMPD_parallel_loop:
   case OMPD_target_parallel_loop:
+  case OMPD_assume:
 Diag(Tok, diag::err_omp_unexpected_directive)
 << 1 << getOpenMPDirectiveName(DKind);
 break;
@@ -3023,6 +3024,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5c759aedf9798..731d839caef03 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..a38380ed4482a
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 0..33c1c6f7c51e7
-

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-21 Thread Julian Brown via cfe-commits



@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}

jtb20 wrote:

Ugh, a bad habit of that editor. I guess I should try to figure out a way of 
stopping it doing that automatically...

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-21 Thread Julian Brown via cfe-commits


https://github.com/jtb20 edited https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-21 Thread Julian Brown via cfe-commits


jtb20 wrote:

> don't you need more code in AST?

Sorry, I don't quite understand the question! Could you elaborate a little 
please?

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-03 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 4337a97fb8982addfd6740db2ff08dfb0398842e Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 22 +
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  3 ++
 7 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 50a872fedebf7c..513af9846aa7ed 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2444,6 +2444,7 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
   case OMPD_target_teams_loop:
   case OMPD_parallel_loop:
   case OMPD_target_parallel_loop:
+  case OMPD_assume:
 Diag(Tok, diag::err_omp_unexpected_directive)
 << 1 << getOpenMPDirectiveName(DKind);
 break;
@@ -3023,6 +3024,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 99528c2a4f1f44..82aaafa91b715f 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3511,7 +3511,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 00..a38380ed4482a3
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 00..33c1c6f7c

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-07-29 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 945c37825742aa6a988ee1bf74b0f3feefbbd906 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This patch supports parsing of "omp assume" directives.  These are
meant to be hints to a compiler' optimisers: as such, it is legitimate
(if not very useful) to ignore them.  This version of the patch
implements new AST nodes for the "assume" directive and the various
clauses it accepts as arguments, and adds new (C++ module) tests for
serialization/deserialization of said nodes.

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

The "assume" directive appears to be distinct from the [[omp::assume]]
annotation.

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.
---
 clang/include/clang-c/Index.h |   4 +
 clang/include/clang/AST/OpenMPClause.h| 174 ++
 clang/include/clang/AST/RecursiveASTVisitor.h |  35 
 clang/include/clang/AST/StmtOpenMP.h  |  30 +++
 clang/include/clang/Basic/OpenMPKinds.h   |   2 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Parse/Parser.h|   4 +
 clang/include/clang/Sema/SemaOpenMP.h |  19 ++
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/OpenMPClause.cpp|  43 +
 clang/lib/AST/StmtOpenMP.cpp  |  17 ++
 clang/lib/AST/StmtPrinter.cpp |   5 +
 clang/lib/AST/StmtProfile.cpp |  18 ++
 clang/lib/Basic/OpenMPKinds.cpp   |   8 +
 clang/lib/CodeGen/CGStmt.cpp  |   3 +
 clang/lib/CodeGen/CGStmtOpenMP.cpp|   7 +-
 clang/lib/CodeGen/CodeGenFunction.h   |   1 +
 clang/lib/Parse/ParseOpenMP.cpp   | 128 +
 clang/lib/Sema/SemaOpenMP.cpp |  72 +++-
 clang/lib/Sema/TreeTransform.h| 127 +
 clang/lib/Serialization/ASTReader.cpp |  50 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 ++
 clang/lib/Serialization/ASTWriter.cpp |  28 +++
 clang/lib/Serialization/ASTWriterStmt.cpp |   6 +
 clang/test/OpenMP/assume_lambda.cpp   |  31 
 clang/test/OpenMP/assume_messages.c   |  30 +++
 clang/test/OpenMP/assume_messages_attr.c  |  30 +++
 .../OpenMP/assume_serialize_deserialize.cpp   |  31 
 .../assume_serialize_deserialize_tmpl.cpp |  72 
 clang/test/OpenMP/assume_template.cpp |  42 +
 clang/tools/libclang/CIndex.cpp   |  21 +++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  |  30 +++
 32 files changed, 1079 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize.cpp
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize_tmpl.cpp
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 115f5ab090f96..4b4adbfb236e7 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2154,6 +2154,10 @@ enum CXCursorKind {
*/
   CXCursor_OMPInterchangeDirective = 308,
 
+  /** OpenMP assume directive.
+   */
+  CXCursor_OMPAssumeDirective = 309,
+
   /** OpenACC Compute Construct.
*/
   CXCursor_OpenACCComputeConstruct = 320,
diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 325a1baa44614..13e3405890ba9 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -29,6 +29,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
@@ -2013,6 +2014,179 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'absent' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume absent()
+/// \endcode
+/// In this example directive '#pragma omp assume' has an 'absent' clause.
+class OMPAbsentClause final : public OMPNoChildClause {
+  llvm::SmallSet DirectiveKinds;
+
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+public:
+  /// Build 'absent' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-07-29 Thread Julian Brown via cfe-commits



@@ -2013,6 +2014,179 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'absent' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume absent()
+/// \endcode
+/// In this example directive '#pragma omp assume' has an 'absent' clause.
+class OMPAbsentClause final : public OMPNoChildClause {
+  llvm::SmallSet DirectiveKinds;
+
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+public:
+  /// Build 'absent' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPAbsentClause(llvm::SmallSet &DKSet,
+  SourceLocation StartLoc, SourceLocation LParenLoc,
+  SourceLocation EndLoc)
+  : OMPNoChildClause(StartLoc, EndLoc), DirectiveKinds(DKSet),
+LParenLoc(LParenLoc) {}
+
+  /// Build an empty clause.
+  OMPAbsentClause() : OMPNoChildClause() {}
+
+  SourceLocation getLParenLoc() { return LParenLoc; }
+
+  void setLParenLoc(SourceLocation S) { LParenLoc = S; }
+
+  llvm::SmallSet &getDirectiveKinds() {
+return DirectiveKinds;
+  }
+
+  void setDirectiveKinds(llvm::SmallSet &DKS) {
+DirectiveKinds = DKS;
+  }
+};
+
+/// This represents the 'contains' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume contains()
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'contains' clause.
+class OMPContainsClause final
+: public OMPNoChildClause {
+  llvm::SmallSet DirectiveKinds;

jtb20 wrote:

To clarify: do you mean that I should use "ompchildren" and encode the 
directive kind list as OMPClauses, rather than use a separate allocation/data 
structure?

That would probably work I guess, but it means adding a pseudo-clause type 
(OMPC_directive_kind or similar), or adding all the possible directive names as 
clause names also (which seems unwieldy for a relatively obscure feature). Is 
one of those along the lines of what you were thinking?

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-07-30 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From a00ae52b9d7200b2a6c90515bf7b251d6b7f0137 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This patch supports parsing of "omp assume" directives.  These are meant
to be hints to a compiler's optimisers: as such, it is legitimate
(if not very useful) to ignore them.  This version of the patch
implements new AST nodes for the "assume" directive and the various
clauses it accepts as arguments, and adds new (C++ module) tests for
serialization/deserialization of said nodes.

The patch now uses tail allocation for the directive kind list.

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

The "assume" directive appears to be distinct from the [[omp::assume]]
annotation.

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.
---
 clang/include/clang-c/Index.h |   4 +
 clang/include/clang/AST/OpenMPClause.h| 229 ++
 clang/include/clang/AST/RecursiveASTVisitor.h |  35 +++
 clang/include/clang/AST/StmtOpenMP.h  |  30 +++
 clang/include/clang/Basic/OpenMPKinds.h   |   2 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Parse/Parser.h|   4 +
 clang/include/clang/Sema/SemaOpenMP.h |  19 ++
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/OpenMPClause.cpp|  78 ++
 clang/lib/AST/StmtOpenMP.cpp  |  17 ++
 clang/lib/AST/StmtPrinter.cpp |   5 +
 clang/lib/AST/StmtProfile.cpp |  18 ++
 clang/lib/Basic/OpenMPKinds.cpp   |   8 +
 clang/lib/CodeGen/CGStmt.cpp  |   3 +
 clang/lib/CodeGen/CGStmtOpenMP.cpp|   7 +-
 clang/lib/CodeGen/CodeGenFunction.h   |   1 +
 clang/lib/Parse/ParseOpenMP.cpp   | 127 ++
 clang/lib/Sema/SemaOpenMP.cpp |  69 ++
 clang/lib/Sema/TreeTransform.h| 127 ++
 clang/lib/Serialization/ASTReader.cpp |  56 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |  28 +++
 clang/lib/Serialization/ASTWriterStmt.cpp |   6 +
 clang/test/OpenMP/assume_lambda.cpp   |  31 +++
 clang/test/OpenMP/assume_messages.c   |  30 +++
 clang/test/OpenMP/assume_messages_attr.c  |  30 +++
 .../OpenMP/assume_serialize_deserialize.cpp   |  31 +++
 .../assume_serialize_deserialize_tmpl.cpp |  72 ++
 clang/test/OpenMP/assume_template.cpp |  42 
 clang/tools/libclang/CIndex.cpp   |  21 ++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  |  33 +++
 32 files changed, 1175 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize.cpp
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize_tmpl.cpp
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 115f5ab090f96..4b4adbfb236e7 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2154,6 +2154,10 @@ enum CXCursorKind {
*/
   CXCursor_OMPInterchangeDirective = 308,
 
+  /** OpenMP assume directive.
+   */
+  CXCursor_OMPAssumeDirective = 309,
+
   /** OpenACC Compute Construct.
*/
   CXCursor_OpenACCComputeConstruct = 320,
diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index b029c72fa7d8f..b970437f6e162 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -342,6 +342,57 @@ template  class OMPVarListClause : public 
OMPClause {
   }
 };
 
+template  class OMPDirectiveListClause : public OMPClause {
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+protected:
+  /// Number of directive kinds listed in the clause
+  unsigned NumKinds;
+
+public:
+  OMPDirectiveListClause(OpenMPClauseKind K, SourceLocation StartLoc,
+ SourceLocation LParenLoc, SourceLocation EndLoc,
+ unsigned NumKinds)
+  : OMPClause(K, StartLoc, EndLoc), LParenLoc(LParenLoc),
+NumKinds(NumKinds) {}
+
+  child_range children() {
+return child_range(child_iterator(), child_iterator());
+  }
+
+  const_child_range children() const {
+return const_child_range(const_child_iterator(), const

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-07-31 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created 
https://github.com/llvm/llvm-project/pull/101305

This patch fixes a couple of cases where Clang aborts with loop nests that are 
being collapsed (via the relevant OpenMP clause) into a new, combined loop.

The problematic cases happen when a variable declared within the loop nest is 
used in the (init, condition, iter) statement of a more deeply-nested loop.  I 
don't think these cases (generally?) fall under the non-rectangular loop nest 
rules as defined in OpenMP 5.0+, but I could be wrong (and anyway, emitting an 
error is better than crashing).

In terms of implementation: the crash happens because (to a first 
approximation) all the loop bounds calculations are pulled out to the start of 
the new, combined loop, but variables declared in the loop nest "haven't been 
seen yet".  I believe there is special handling for iteration variables 
declared in "for" init statements, but not for variables declared elsewhere in 
the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can cause a 
crash.  This is slightly awkward because at the point where we want to do the 
diagnosis (SemaOpenMP.cpp), we don't have scope information readily available.  
Instead we "manually" scan through the AST of the loop nest looking for var 
decls (ForVarDeclFinder), then we ensure we're not using any of those in loop 
control subexprs (ForSubExprChecker). All that is only done when we have a 
"collapse" clause.

Range-for loops can also cause crashes at present without this patch, so are 
handled too.

>From 56d5d7797929d8bc81bf394a46c97b9bf645744e Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:21:01 -0500
Subject: [PATCH] [clang][OpenMP] Diagnose badly-formed collapsed imperfect
 loop nests (#60678)

This patch fixes a couple of cases where Clang aborts with loop nests
that are being collapsed (via the relevant OpenMP clause) into a new,
combined loop.

The problematic cases happen when a variable declared within the
loop nest is used in the (init, condition, iter) statement of a more
deeply-nested loop.  I don't think these cases (generally?) fall under
the non-rectangular loop nest rules as defined in OpenMP 5.0+, but I
could be wrong (and anyway, emitting an error is better than crashing).

In terms of implementation: the crash happens because (to a first
approximation) all the loop bounds calculations are pulled out to the
start of the new, combined loop, but variables declared in the loop nest
"haven't been seen yet".  I believe there is special handling for
iteration variables declared in "for" init statements, but not for
variables declared elsewhere in the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can
cause a crash.  This is slightly awkward because at the point where we
want to do the diagnosis (SemaOpenMP.cpp), we don't have scope information
readily available.  Instead we "manually" scan through the AST of the
loop nest looking for var decls (ForVarDeclFinder), then we ensure we're
not using any of those in loop control subexprs (ForSubExprChecker).
All that is only done when we have a "collapse" clause.

Range-for loops can also cause crashes at present without this patch,
so are handled too.
---
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +
 clang/lib/AST/StmtOpenMP.cpp  |   1 +
 clang/lib/Sema/SemaOpenMP.cpp | 132 --
 clang/test/OpenMP/loop_collapse_1.c   |  40 ++
 clang/test/OpenMP/loop_collapse_2.cpp |  80 +++
 5 files changed, 247 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/OpenMP/loop_collapse_1.c
 create mode 100644 clang/test/OpenMP/loop_collapse_2.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..beb78eb0a4ef4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11152,6 +11152,8 @@ def err_omp_loop_diff_cxx : Error<
   "upper and lower loop bounds">;
 def err_omp_loop_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP for loop">;
+def err_omp_loop_bad_collapse_var : Error<
+  "cannot use variable %1 in collapsed imperfectly-nested loop 
%select{init|condition|increment}0 statement">;
 def err_omp_simd_region_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP simd region">;
 def warn_omp_loop_64_bit_var : Warning<
diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index 451a9fe9fe3d2..75ea55c99dfc5 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 
 using namespace clang;
 using namespace llvm::omp;
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpen

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-07-31 Thread Julian Brown via cfe-commits


jtb20 wrote:

@jdoerfert @alexey-bataev 

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-07-31 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/101305

>From eb60fc7c8d02877d2f8de73057c1153444cc740c Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:21:01 -0500
Subject: [PATCH] [clang][OpenMP] Diagnose badly-formed collapsed imperfect
 loop nests (#60678)

This patch fixes a couple of cases where Clang aborts with loop nests
that are being collapsed (via the relevant OpenMP clause) into a new,
combined loop.

The problematic cases happen when a variable declared within the
loop nest is used in the (init, condition, iter) statement of a more
deeply-nested loop.  I don't think these cases (generally?) fall under
the non-rectangular loop nest rules as defined in OpenMP 5.0+, but I
could be wrong (and anyway, emitting an error is better than crashing).

In terms of implementation: the crash happens because (to a first
approximation) all the loop bounds calculations are pulled out to the
start of the new, combined loop, but variables declared in the loop nest
"haven't been seen yet".  I believe there is special handling for
iteration variables declared in "for" init statements, but not for
variables declared elsewhere in the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can
cause a crash.  This is slightly awkward because at the point where we
want to do the diagnosis (SemaOpenMP.cpp), we don't have scope information
readily available.  Instead we "manually" scan through the AST of the
loop nest looking for var decls (ForVarDeclFinder), then we ensure we're
not using any of those in loop control subexprs (ForSubExprChecker).
All that is only done when we have a "collapse" clause.

Range-for loops can also cause crashes at present without this patch,
so are handled too.
---
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +
 clang/lib/AST/StmtOpenMP.cpp  |   3 +-
 clang/lib/Sema/SemaOpenMP.cpp | 132 --
 clang/test/OpenMP/loop_collapse_1.c   |  40 ++
 clang/test/OpenMP/loop_collapse_2.cpp |  80 +++
 5 files changed, 248 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/OpenMP/loop_collapse_1.c
 create mode 100644 clang/test/OpenMP/loop_collapse_2.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..beb78eb0a4ef4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11152,6 +11152,8 @@ def err_omp_loop_diff_cxx : Error<
   "upper and lower loop bounds">;
 def err_omp_loop_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP for loop">;
+def err_omp_loop_bad_collapse_var : Error<
+  "cannot use variable %1 in collapsed imperfectly-nested loop 
%select{init|condition|increment}0 statement">;
 def err_omp_simd_region_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP simd region">;
 def warn_omp_loop_64_bit_var : Warning<
diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index 451a9fe9fe3d2..e41c26bb60252 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -10,8 +10,9 @@
 //
 
//===--===//
 
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 
 using namespace clang;
 using namespace llvm::omp;
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 4f50efda155fb..e78af5cc7ab0a 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
@@ -7668,6 +7669,47 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+class ForSubExprChecker : public RecursiveASTVisitor {
+  const llvm::SmallSet *CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallSet *CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls), ForbiddenVar(nullptr) {}
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+ValueDecl *VD = E->getDecl();
+if (!isa(VD))
+  return true;
+VarDecl *V = VD->getPotentiallyDecomposedVarDecl();
+if (V->getType()->isReferenceType()) {
+  VarDecl *VD = V->getDefinition();
+  if (VD->hasInit()) {
+Expr *I = VD->getInit();
+DeclRefExpr *DRE = dyn_cast(I);
+if (!DRE)
+  return true;
+V = DRE->getDecl()->getPotentiallyDecomposedVarDecl();
+  }
+}
+Decl *Canon = V->getC

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-07-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From fc4e93006a8a1def0fabdc358ce85fac267e5a63 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This patch supports parsing of "omp assume" directives.  These are
meant to be hints to a compiler' optimisers: as such, it is legitimate
(if not very useful) to ignore them.  This version of the patch
implements new AST nodes for the "assume" directive and the various
clauses it accepts as arguments, and adds new (C++ module) tests for
serialization/deserialization of said nodes.

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

The "assume" directive appears to be distinct from the [[omp::assume]]
annotation.

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.
---
 clang/include/clang-c/Index.h |   4 +
 clang/include/clang/AST/OpenMPClause.h| 178 ++
 clang/include/clang/AST/RecursiveASTVisitor.h |  35 
 clang/include/clang/AST/StmtOpenMP.h  |  31 +++
 clang/include/clang/Basic/OpenMPKinds.h   |   2 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Parse/Parser.h|   6 +
 clang/include/clang/Sema/SemaOpenMP.h |  18 ++
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/OpenMPClause.cpp|  43 +
 clang/lib/AST/StmtOpenMP.cpp  |  16 ++
 clang/lib/AST/StmtPrinter.cpp |   5 +
 clang/lib/AST/StmtProfile.cpp |  18 ++
 clang/lib/Basic/OpenMPKinds.cpp   |   8 +
 clang/lib/CodeGen/CGStmt.cpp  |   3 +
 clang/lib/CodeGen/CGStmtOpenMP.cpp|   7 +-
 clang/lib/CodeGen/CodeGenFunction.h   |   1 +
 clang/lib/Parse/ParseOpenMP.cpp   | 126 +
 clang/lib/Sema/SemaOpenMP.cpp |  70 ++-
 clang/lib/Sema/TreeTransform.h| 122 
 clang/lib/Serialization/ASTReader.cpp |  50 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 ++
 clang/lib/Serialization/ASTWriter.cpp |  28 +++
 clang/lib/Serialization/ASTWriterStmt.cpp |   6 +
 clang/test/OpenMP/assume_lambda.cpp   |  31 +++
 clang/test/OpenMP/assume_messages.c   |  30 +++
 clang/test/OpenMP/assume_messages_attr.c  |  30 +++
 .../OpenMP/assume_serialize_deserialize.cpp   |  31 +++
 .../assume_serialize_deserialize_tmpl.cpp |  72 +++
 clang/test/OpenMP/assume_template.cpp |  42 +
 clang/tools/libclang/CIndex.cpp   |  19 ++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  |  30 +++
 32 files changed, 1073 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize.cpp
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize_tmpl.cpp
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 115f5ab090f96..4b4adbfb236e7 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2154,6 +2154,10 @@ enum CXCursorKind {
*/
   CXCursor_OMPInterchangeDirective = 308,
 
+  /** OpenMP assume directive.
+   */
+  CXCursor_OMPAssumeDirective = 309,
+
   /** OpenACC Compute Construct.
*/
   CXCursor_OpenACCComputeConstruct = 320,
diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 325a1baa44614..fb1157867918c 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -32,6 +32,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Frontend/OpenMP/OMPAssume.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPContext.h"
@@ -2013,6 +2014,183 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'absent' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume absent()
+/// \endcode
+/// In this example directive '#pragma omp assume' has an 'absent' clause.
+class OMPAbsentClause final
+: public OMPNoChildClause {
+  llvm::SmallSet DirectiveKinds;
+
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+public:
+  /// Build 'absent' clause.
+  ///
+  /// \param StartLoc Starting location o

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-07-20 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From f6addc40ef18c3ca28ee79303bace08d30dc9817 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This patch supports parsing of "omp assume" directives.  These are
meant to be hints to a compiler' optimisers: as such, it is legitimate
(if not very useful) to ignore them.  This version of the patch
implements new AST nodes for the "assume" directive and the various
clauses it accepts as arguments, and adds new (C++ module) tests for
serialization/deserialization of said nodes.

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

The "assume" directive appears to be distinct from the [[omp::assume]]
annotation.

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.
---
 clang/include/clang-c/Index.h |   4 +
 clang/include/clang/AST/OpenMPClause.h| 174 ++
 clang/include/clang/AST/RecursiveASTVisitor.h |  35 
 clang/include/clang/AST/StmtOpenMP.h  |  30 +++
 clang/include/clang/Basic/OpenMPKinds.h   |   2 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Parse/Parser.h|   4 +
 clang/include/clang/Sema/SemaOpenMP.h |  19 ++
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/OpenMPClause.cpp|  43 +
 clang/lib/AST/StmtOpenMP.cpp  |  17 ++
 clang/lib/AST/StmtPrinter.cpp |   5 +
 clang/lib/AST/StmtProfile.cpp |  18 ++
 clang/lib/Basic/OpenMPKinds.cpp   |   8 +
 clang/lib/CodeGen/CGStmt.cpp  |   3 +
 clang/lib/CodeGen/CGStmtOpenMP.cpp|   7 +-
 clang/lib/CodeGen/CodeGenFunction.h   |   1 +
 clang/lib/Parse/ParseOpenMP.cpp   | 128 +
 clang/lib/Sema/SemaOpenMP.cpp |  72 +++-
 clang/lib/Sema/TreeTransform.h| 127 +
 clang/lib/Serialization/ASTReader.cpp |  50 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 ++
 clang/lib/Serialization/ASTWriter.cpp |  28 +++
 clang/lib/Serialization/ASTWriterStmt.cpp |   6 +
 clang/test/OpenMP/assume_lambda.cpp   |  31 
 clang/test/OpenMP/assume_messages.c   |  30 +++
 clang/test/OpenMP/assume_messages_attr.c  |  30 +++
 .../OpenMP/assume_serialize_deserialize.cpp   |  31 
 .../assume_serialize_deserialize_tmpl.cpp |  72 
 clang/test/OpenMP/assume_template.cpp |  42 +
 clang/tools/libclang/CIndex.cpp   |  21 +++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  |  30 +++
 32 files changed, 1079 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize.cpp
 create mode 100644 clang/test/OpenMP/assume_serialize_deserialize_tmpl.cpp
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 115f5ab090f96..4b4adbfb236e7 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -2154,6 +2154,10 @@ enum CXCursorKind {
*/
   CXCursor_OMPInterchangeDirective = 308,
 
+  /** OpenMP assume directive.
+   */
+  CXCursor_OMPAssumeDirective = 309,
+
   /** OpenACC Compute Construct.
*/
   CXCursor_OpenACCComputeConstruct = 320,
diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 325a1baa44614..13e3405890ba9 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -29,6 +29,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerIntPair.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
@@ -2013,6 +2014,179 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'absent' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume absent()
+/// \endcode
+/// In this example directive '#pragma omp assume' has an 'absent' clause.
+class OMPAbsentClause final : public OMPNoChildClause {
+  llvm::SmallSet DirectiveKinds;
+
+  /// Location of '('.
+  SourceLocation LParenLoc;
+
+public:
+  /// Build 'absent' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits



@@ -7668,6 +7669,47 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+class ForSubExprChecker : public RecursiveASTVisitor {
+  const llvm::SmallSet *CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallSet *CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls), ForbiddenVar(nullptr) {}
+
+  bool shouldVisitImplicitCode() const { return true; }

jtb20 wrote:

Which bit do you think is unused? The next revision includes a comment on the 
shouldVisitImplicitCode definition (which is needed).

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits



@@ -7668,6 +7669,47 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+class ForSubExprChecker : public RecursiveASTVisitor {
+  const llvm::SmallSet *CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallSet *CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls), ForbiddenVar(nullptr) {}
+
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+ValueDecl *VD = E->getDecl();
+if (!isa(VD))
+  return true;
+VarDecl *V = VD->getPotentiallyDecomposedVarDecl();
+if (V->getType()->isReferenceType()) {
+  VarDecl *VD = V->getDefinition();
+  if (VD->hasInit()) {
+Expr *I = VD->getInit();
+DeclRefExpr *DRE = dyn_cast(I);
+if (!DRE)
+  return true;
+V = DRE->getDecl()->getPotentiallyDecomposedVarDecl();
+  }
+}
+Decl *Canon = V->getCanonicalDecl();
+if (CollapsedLoopVarDecls->contains(Canon)) {
+  ForbiddenVar = V;
+  ErrLoc = E->getSourceRange();
+  return false;
+}
+
+return true;
+  }
+
+  VarDecl *getForbiddenVar() { return ForbiddenVar; }
+  SourceRange &getErrRange() { return ErrLoc; }

jtb20 wrote:

OK but then getErrRange needs to return SourceRange by value, I think.

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits



@@ -7668,6 +7669,47 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+class ForSubExprChecker : public RecursiveASTVisitor {
+  const llvm::SmallSet *CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallSet *CollapsedLoopVarDecls)

jtb20 wrote:

There isn't a SmallSetImpl, so I've changed all these to SmallPtrSet{,Impl} 
instead.

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/101305

>From 2d318c6504b43d8a9521dc5567c1da4d6cd986a4 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:21:01 -0500
Subject: [PATCH] [clang][OpenMP] Diagnose badly-formed collapsed imperfect
 loop nests (#60678)

This patch fixes a couple of cases where Clang aborts with loop nests
that are being collapsed (via the relevant OpenMP clause) into a new,
combined loop.

The problematic cases happen when a variable declared within the
loop nest is used in the (init, condition, iter) statement of a more
deeply-nested loop.  I don't think these cases (generally?) fall under
the non-rectangular loop nest rules as defined in OpenMP 5.0+, but I
could be wrong (and anyway, emitting an error is better than crashing).

In terms of implementation: the crash happens because (to a first
approximation) all the loop bounds calculations are pulled out to the
start of the new, combined loop, but variables declared in the loop nest
"haven't been seen yet".  I believe there is special handling for
iteration variables declared in "for" init statements, but not for
variables declared elsewhere in the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can
cause a crash.  This is slightly awkward because at the point where we
want to do the diagnosis (SemaOpenMP.cpp), we don't have scope information
readily available.  Instead we "manually" scan through the AST of the
loop nest looking for var decls (ForVarDeclFinder), then we ensure we're
not using any of those in loop control subexprs (ForSubExprChecker).
All that is only done when we have a "collapse" clause.

Range-for loops can also cause crashes at present without this patch,
so are handled too.
---
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +
 clang/lib/AST/StmtOpenMP.cpp  |   3 +-
 clang/lib/Sema/SemaOpenMP.cpp | 140 +-
 clang/test/OpenMP/loop_collapse_1.c   |  40 +
 clang/test/OpenMP/loop_collapse_2.cpp |  80 ++
 5 files changed, 256 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/OpenMP/loop_collapse_1.c
 create mode 100644 clang/test/OpenMP/loop_collapse_2.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..beb78eb0a4ef4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11152,6 +11152,8 @@ def err_omp_loop_diff_cxx : Error<
   "upper and lower loop bounds">;
 def err_omp_loop_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP for loop">;
+def err_omp_loop_bad_collapse_var : Error<
+  "cannot use variable %1 in collapsed imperfectly-nested loop 
%select{init|condition|increment}0 statement">;
 def err_omp_simd_region_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP simd region">;
 def warn_omp_loop_64_bit_var : Warning<
diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index 451a9fe9fe3d2..e41c26bb60252 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -10,8 +10,9 @@
 //
 
//===--===//
 
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 
 using namespace clang;
 using namespace llvm::omp;
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 4f50efda155fb..631dc2a33c3a3 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
@@ -7668,6 +7669,52 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+/// Scan an AST subtree, checking that no decls in the CollapsedLoopVarDecls
+/// set are referenced.  Used for verifying loop nest structure before
+/// performing a loop collapse operation.
+class ForSubExprChecker final : public RecursiveASTVisitor {
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar = nullptr;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls) {}
+
+  // We want to visit implicit code, i.e. synthetic initialisation statements
+  // created during range-for lowering.
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+ValueDecl *VD = E->getDecl();
+if (!isa(VD))
+  return true;
+VarDecl *V = VD->getPotentiallyDecomposedVarDecl();
+if (V->getType

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits


jtb20 wrote:

OpenMPIterationSpaceChecker is still passed a pointer to CollapsedLoopDecls, 
because one caller passes a nullptr, and we don't want to do the analysis in 
that case.

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/101305

>From 32c370577279b6ba9a5947b8936c8852ae809e07 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:21:01 -0500
Subject: [PATCH] [clang][OpenMP] Diagnose badly-formed collapsed imperfect
 loop nests (#60678)

This patch fixes a couple of cases where Clang aborts with loop nests
that are being collapsed (via the relevant OpenMP clause) into a new,
combined loop.

The problematic cases happen when a variable declared within the
loop nest is used in the (init, condition, iter) statement of a more
deeply-nested loop.  I don't think these cases (generally?) fall under
the non-rectangular loop nest rules as defined in OpenMP 5.0+, but I
could be wrong (and anyway, emitting an error is better than crashing).

In terms of implementation: the crash happens because (to a first
approximation) all the loop bounds calculations are pulled out to the
start of the new, combined loop, but variables declared in the loop nest
"haven't been seen yet".  I believe there is special handling for
iteration variables declared in "for" init statements, but not for
variables declared elsewhere in the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can
cause a crash.  This is slightly awkward because at the point where we
want to do the diagnosis (SemaOpenMP.cpp), we don't have scope information
readily available.  Instead we "manually" scan through the AST of the
loop nest looking for var decls (ForVarDeclFinder), then we ensure we're
not using any of those in loop control subexprs (ForSubExprChecker).
All that is only done when we have a "collapse" clause.

Range-for loops can also cause crashes at present without this patch,
so are handled too.
---
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +
 clang/lib/AST/StmtOpenMP.cpp  |   3 +-
 clang/lib/Sema/SemaOpenMP.cpp | 141 +-
 clang/test/OpenMP/loop_collapse_1.c   |  40 +
 clang/test/OpenMP/loop_collapse_2.cpp |  80 ++
 5 files changed, 257 insertions(+), 9 deletions(-)
 create mode 100644 clang/test/OpenMP/loop_collapse_1.c
 create mode 100644 clang/test/OpenMP/loop_collapse_2.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..beb78eb0a4ef4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11152,6 +11152,8 @@ def err_omp_loop_diff_cxx : Error<
   "upper and lower loop bounds">;
 def err_omp_loop_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP for loop">;
+def err_omp_loop_bad_collapse_var : Error<
+  "cannot use variable %1 in collapsed imperfectly-nested loop 
%select{init|condition|increment}0 statement">;
 def err_omp_simd_region_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP simd region">;
 def warn_omp_loop_64_bit_var : Warning<
diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index 451a9fe9fe3d2..e41c26bb60252 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -10,8 +10,9 @@
 //
 
//===--===//
 
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 
 using namespace clang;
 using namespace llvm::omp;
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 4f50efda155fb..c52efd78a2379 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
@@ -7668,6 +7669,52 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+/// Scan an AST subtree, checking that no decls in the CollapsedLoopVarDecls
+/// set are referenced.  Used for verifying loop nest structure before
+/// performing a loop collapse operation.
+class ForSubExprChecker final : public RecursiveASTVisitor {
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar = nullptr;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls) {}
+
+  // We want to visit implicit code, i.e. synthetic initialisation statements
+  // created during range-for lowering.
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+ValueDecl *VD = E->getDecl();
+if (!isa(VD))
+  return true;
+VarDecl *V = VD->getPotentiallyDecomposedVarDecl();
+if (V->getType

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-01 Thread Julian Brown via cfe-commits

jtb20 wrote:

> > OpenMPIterationSpaceChecker is still passed a pointer to 
> > CollapsedLoopDecls, because one caller passes a nullptr, and we don't want 
> > to do the analysis in that case.
> 
> Still pass by reference, just pass empty where it is not required

So the new pushed version does that, sort of -- but I'm not sure it's 
unambiguously better. (The "empty" member is a predicate for SmallPtrSet, not a 
value representing an empty set. Did you mean something else?)

Otherwise, std::optional might be a possibility, but it can't be used with 
references, IIUC.

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-01 Thread Julian Brown via cfe-commits



@@ -2013,6 +2064,184 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'absent' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume absent()
+/// \endcode
+/// In this example directive '#pragma omp assume' has an 'absent' clause.
+class OMPAbsentClause final
+: public OMPDirectiveListClause,
+  private llvm::TrailingObjects {
+  friend OMPDirectiveListClause;
+  friend TrailingObjects;
+
+public:
+  /// Build 'absent' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param EndLoc Ending location of the clause.
+  /// \param NumKinds Number of directive kinds listed in the clause.
+  OMPAbsentClause(SourceLocation StartLoc, SourceLocation LParenLoc,
+  SourceLocation EndLoc, unsigned NumKinds)
+  : OMPDirectiveListClause(
+llvm::omp::OMPC_absent, StartLoc, LParenLoc, EndLoc, NumKinds) {}
+
+  /// Build an empty clause.
+  OMPAbsentClause(unsigned NumKinds)
+  : OMPDirectiveListClause(
+llvm::omp::OMPC_absent, SourceLocation(), SourceLocation(),
+SourceLocation(), NumKinds) {}
+
+  static OMPAbsentClause *Create(const ASTContext &C,
+ ArrayRef DKVec,
+ SourceLocation Loc, SourceLocation LLoc,
+ SourceLocation RLoc);
+
+  static OMPAbsentClause *CreateEmpty(const ASTContext &C, unsigned NumKinds);
+
+  static bool classof(const OMPClause *C) {
+return C->getClauseKind() == llvm::omp::OMPC_absent;
+  }
+};
+
+/// This represents the 'contains' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume contains()
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'contains' clause.
+class OMPContainsClause final
+: public OMPDirectiveListClause,
+  private llvm::TrailingObjects {
+  friend OMPDirectiveListClause;
+  friend TrailingObjects;
+
+public:
+  /// Build 'contains' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param LParenLoc Location of '('.
+  /// \param EndLoc Ending location of the clause.
+  /// \param NumKinds Number of directive kinds listed in the clause.
+  OMPContainsClause(SourceLocation StartLoc, SourceLocation LParenLoc,
+SourceLocation EndLoc, unsigned NumKinds)
+  : OMPDirectiveListClause(
+llvm::omp::OMPC_contains, StartLoc, LParenLoc, EndLoc, NumKinds) {}
+
+  /// Build an empty clause.
+  OMPContainsClause(unsigned NumKinds)
+  : OMPDirectiveListClause(
+llvm::omp::OMPC_contains, SourceLocation(), SourceLocation(),
+SourceLocation(), NumKinds) {}
+
+  static OMPContainsClause *Create(const ASTContext &C,
+   ArrayRef DKVec,
+   SourceLocation Loc, SourceLocation LLoc,
+   SourceLocation RLoc);
+
+  static OMPContainsClause *CreateEmpty(const ASTContext &C, unsigned 
NumKinds);
+
+  static bool classof(const OMPClause *C) {
+return C->getClauseKind() == llvm::omp::OMPC_contains;
+  }
+};
+
+/// This represents the 'holds' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume holds()
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'holds' clause.
+class OMPHoldsClause final
+: public OMPOneStmtClause {
+  friend class OMPClauseReader;
+
+public:
+  /// Build 'holds' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPHoldsClause(Expr *E, SourceLocation StartLoc, SourceLocation LParenLoc,
+ SourceLocation EndLoc)
+  : OMPOneStmtClause(E, StartLoc, LParenLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPHoldsClause() : OMPOneStmtClause() {}
+
+  Expr *getExpr() const { return getStmtAs(); }
+  void setExpr(Expr *E) { setStmt(E); }
+};
+
+/// This represents the 'no_openmp' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume no_openmp
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'no_openmp' clause.
+class OMPNoOpenMPClause final
+: public OMPNoChildClause {
+public:
+  /// Build 'no_openmp' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPNoOpenMPClause(SourceLocation StartLoc, SourceLocation EndLoc)
+  : OMPNoChildClause(StartLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPNoOpenMPClause() : OMPNoChildClause() {}
+};
+
+/// This represents the 'no_openmp_routines' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume no_openmp_routines
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'no_openmp_routines'
+/// clause.
+class OMPNoOpe

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-01 Thread Julian Brown via cfe-commits



@@ -342,6 +342,57 @@ template  class OMPVarListClause : public 
OMPClause {
   }
 };
 
+template  class OMPDirectiveListClause : public OMPClause {

jtb20 wrote:

The idea was to factor out common functionality shared by absent/contains 
clauses. It doesn't do a huge amount, admittedly, but I think it's still better 
than copy/pasting a bunch of code for the absent/contains clause classes. 
(OMPNoChildClause, OMPOneStmtClause are similar in intent though perhaps wider 
in applicability).

It should probably have a comment, if that's what you meant!

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-01 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 73257cd3390361fa71735a39290dd47427916734 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Thu, 1 Aug 2024 14:49:28 -0500
Subject: [PATCH 1/8] [OpenMP][clang] Add 'holds' clause for 'omp assume'
 directive

---
 clang/include/clang/AST/OpenMPClause.h| 27 +++
 clang/include/clang/AST/RecursiveASTVisitor.h |  5 
 clang/include/clang/Sema/SemaOpenMP.h |  4 +++
 clang/lib/AST/OpenMPClause.cpp|  6 +
 clang/lib/AST/StmtProfile.cpp |  2 ++
 clang/lib/Parse/ParseOpenMP.cpp   |  6 +
 clang/lib/Sema/SemaOpenMP.cpp |  9 +++
 clang/lib/Sema/TreeTransform.h| 17 
 clang/lib/Serialization/ASTReader.cpp |  8 ++
 clang/lib/Serialization/ASTWriter.cpp |  5 
 clang/tools/libclang/CIndex.cpp   |  2 ++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  | 10 +++
 12 files changed, 101 insertions(+)

diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index b029c72fa7d8f..47c2fd4400d52 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -2013,6 +2013,33 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'holds' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume holds()
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'holds' clause.
+class OMPHoldsClause final
+: public OMPOneStmtClause {
+  friend class OMPClauseReader;
+
+public:
+  /// Build 'holds' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPHoldsClause(Expr *E, SourceLocation StartLoc, SourceLocation LParenLoc,
+ SourceLocation EndLoc)
+  : OMPOneStmtClause(E, StartLoc, LParenLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPHoldsClause() : OMPOneStmtClause() {}
+
+  Expr *getExpr() const { return getStmtAs(); }
+  void setExpr(Expr *E) { setStmt(E); }
+};
+
 /// This represents 'read' clause in the '#pragma omp atomic' directive.
 ///
 /// \code
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index dcf5dbf449f8b..73ece1e72bbf1 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -3480,6 +3480,11 @@ bool 
RecursiveASTVisitor::VisitOMPAcqRelClause(OMPAcqRelClause *) {
   return true;
 }
 
+template 
+bool RecursiveASTVisitor::VisitOMPHoldsClause(OMPHoldsClause *) {
+  return true;
+}
+
 template 
 bool RecursiveASTVisitor::VisitOMPAcquireClause(OMPAcquireClause *) {
   return true;
diff --git a/clang/include/clang/Sema/SemaOpenMP.h 
b/clang/include/clang/Sema/SemaOpenMP.h
index aa61dae9415e2..7c793cf3077a6 100644
--- a/clang/include/clang/Sema/SemaOpenMP.h
+++ b/clang/include/clang/Sema/SemaOpenMP.h
@@ -940,6 +940,10 @@ class SemaOpenMP : public SemaBase {
  SourceLocation StartLoc,
  SourceLocation LParenLoc,
  SourceLocation EndLoc);
+  /// Called on well-formed 'holds' clause.
+  OMPClause *ActOnOpenMPHoldsClause(Expr *E, SourceLocation StartLoc,
+SourceLocation LParenLoc,
+SourceLocation EndLoc);
 
   OMPClause *ActOnOpenMPSingleExprWithArgClause(
   OpenMPClauseKind Kind, ArrayRef Arguments, Expr *Expr,
diff --git a/clang/lib/AST/OpenMPClause.cpp b/clang/lib/AST/OpenMPClause.cpp
index 042a5df5906ca..e83c81136c34b 100644
--- a/clang/lib/AST/OpenMPClause.cpp
+++ b/clang/lib/AST/OpenMPClause.cpp
@@ -1937,6 +1937,12 @@ void OMPClausePrinter::VisitOMPFailClause(OMPFailClause 
*Node) {
   }
 }
 
+void OMPClausePrinter::VisitOMPHoldsClause(OMPHoldsClause *Node) {
+  OS << "holds(";
+  Node->getExpr()->printPretty(OS, nullptr, Policy, 0);
+  OS << ")";
+}
+
 void OMPClausePrinter::VisitOMPSeqCstClause(OMPSeqCstClause *) {
   OS << "seq_cst";
 }
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index f1e723b4242ee..97e4b3749f94c 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -584,6 +584,8 @@ void OMPClauseProfiler::VisitOMPCompareClause(const 
OMPCompareClause *) {}
 
 void OMPClauseProfiler::VisitOMPFailClause(const OMPFailClause *) {}
 
+void OMPClauseProfiler::VisitOMPHoldsClause(const OMPHoldsClause *) {}
+
 void OMPClauseProfiler::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseProfiler::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index e975e96c5c7e4..afcbc93696de2 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -3206,6 +3206,9

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-01 Thread Julian Brown via cfe-commits


jtb20 wrote:

The compiler builds for each intermediate step, but the result isn't 
necessarily useful until the 'omp assume' patch.

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-02 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/101305

>From 0d9ee78eb214135b10ecb5258358728e0fc626e1 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:21:01 -0500
Subject: [PATCH] [clang][OpenMP] Diagnose badly-formed collapsed imperfect
 loop nests (#60678)

This patch fixes a couple of cases where Clang aborts with loop nests
that are being collapsed (via the relevant OpenMP clause) into a new,
combined loop.

The problematic cases happen when a variable declared within the
loop nest is used in the (init, condition, iter) statement of a more
deeply-nested loop.  I don't think these cases (generally?) fall under
the non-rectangular loop nest rules as defined in OpenMP 5.0+, but I
could be wrong (and anyway, emitting an error is better than crashing).

In terms of implementation: the crash happens because (to a first
approximation) all the loop bounds calculations are pulled out to the
start of the new, combined loop, but variables declared in the loop nest
"haven't been seen yet".  I believe there is special handling for
iteration variables declared in "for" init statements, but not for
variables declared elsewhere in the "imperfect" parts of a loop nest.

So, this patch tries to diagnose the troublesome cases before they can
cause a crash.  This is slightly awkward because at the point where we
want to do the diagnosis (SemaOpenMP.cpp), we don't have scope information
readily available.  Instead we "manually" scan through the AST of the
loop nest looking for var decls (ForVarDeclFinder), then we ensure we're
not using any of those in loop control subexprs (ForSubExprChecker).
All that is only done when we have a "collapse" clause.

Range-for loops can also cause crashes at present without this patch,
so are handled too.
---
 .../clang/Basic/DiagnosticSemaKinds.td|   2 +
 clang/lib/Sema/SemaOpenMP.cpp | 141 +-
 clang/test/OpenMP/loop_collapse_1.c   |  40 +
 clang/test/OpenMP/loop_collapse_2.cpp |  80 ++
 4 files changed, 255 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/OpenMP/loop_collapse_1.c
 create mode 100644 clang/test/OpenMP/loop_collapse_2.cpp

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 581434d33c5c9..beb78eb0a4ef4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11152,6 +11152,8 @@ def err_omp_loop_diff_cxx : Error<
   "upper and lower loop bounds">;
 def err_omp_loop_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP for loop">;
+def err_omp_loop_bad_collapse_var : Error<
+  "cannot use variable %1 in collapsed imperfectly-nested loop 
%select{init|condition|increment}0 statement">;
 def err_omp_simd_region_cannot_use_stmt : Error<
   "'%0' statement cannot be used in OpenMP simd region">;
 def warn_omp_loop_64_bit_var : Warning<
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 4f50efda155fb..f52a5715aa19e 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/StmtVisitor.h"
@@ -7668,6 +7669,52 @@ struct LoopIterationSpace final {
   Expr *FinalCondition = nullptr;
 };
 
+/// Scan an AST subtree, checking that no decls in the CollapsedLoopVarDecls
+/// set are referenced.  Used for verifying loop nest structure before
+/// performing a loop collapse operation.
+class ForSubExprChecker final : public RecursiveASTVisitor {
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls;
+  VarDecl *ForbiddenVar = nullptr;
+  SourceRange ErrLoc;
+
+public:
+  explicit ForSubExprChecker(
+  const llvm::SmallPtrSetImpl &CollapsedLoopVarDecls)
+  : CollapsedLoopVarDecls(CollapsedLoopVarDecls) {}
+
+  // We want to visit implicit code, i.e. synthetic initialisation statements
+  // created during range-for lowering.
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+ValueDecl *VD = E->getDecl();
+if (!isa(VD))
+  return true;
+VarDecl *V = VD->getPotentiallyDecomposedVarDecl();
+if (V->getType()->isReferenceType()) {
+  VarDecl *VD = V->getDefinition();
+  if (VD->hasInit()) {
+Expr *I = VD->getInit();
+DeclRefExpr *DRE = dyn_cast(I);
+if (!DRE)
+  return true;
+V = DRE->getDecl()->getPotentiallyDecomposedVarDecl();
+  }
+}
+Decl *Canon = V->getCanonicalDecl();
+if (CollapsedLoopVarDecls.contains(Canon)) {
+  ForbiddenVar = V;
+  ErrLoc = E->getSourceRange();
+  return false;
+}
+
+return true;
+  }
+
+  VarDecl *getForbiddenVar() const { return Forbi

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-02 Thread Julian Brown via cfe-commits


jtb20 wrote:

Thank you! I don't have commit access, so do you mind committing it for me 
please?

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-02 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From b3d00ab3f6457094a8ccf4df03b72976e96cd307 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Thu, 1 Aug 2024 14:49:28 -0500
Subject: [PATCH 1/8] [OpenMP][clang] Add 'holds' clause for 'omp assume'
 directive

---
 clang/include/clang/AST/OpenMPClause.h| 27 +++
 clang/include/clang/AST/RecursiveASTVisitor.h |  5 
 clang/include/clang/Sema/SemaOpenMP.h |  4 +++
 clang/lib/AST/OpenMPClause.cpp|  6 +
 clang/lib/AST/StmtProfile.cpp |  2 ++
 clang/lib/Parse/ParseOpenMP.cpp   |  6 +
 clang/lib/Sema/SemaOpenMP.cpp |  9 +++
 clang/lib/Sema/TreeTransform.h| 17 
 clang/lib/Serialization/ASTReader.cpp |  8 ++
 clang/lib/Serialization/ASTWriter.cpp |  5 
 clang/tools/libclang/CIndex.cpp   |  2 ++
 llvm/include/llvm/Frontend/OpenMP/OMP.td  | 10 +++
 12 files changed, 101 insertions(+)

diff --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index b029c72fa7d8f..47c2fd4400d52 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -2013,6 +2013,33 @@ class OMPMergeableClause : public OMPClause {
   }
 };
 
+/// This represents the 'holds' clause in the '#pragma omp assume'
+/// directive.
+///
+/// \code
+/// #pragma omp assume holds()
+/// \endcode
+/// In this example directive '#pragma omp assume' has a 'holds' clause.
+class OMPHoldsClause final
+: public OMPOneStmtClause {
+  friend class OMPClauseReader;
+
+public:
+  /// Build 'holds' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPHoldsClause(Expr *E, SourceLocation StartLoc, SourceLocation LParenLoc,
+ SourceLocation EndLoc)
+  : OMPOneStmtClause(E, StartLoc, LParenLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPHoldsClause() : OMPOneStmtClause() {}
+
+  Expr *getExpr() const { return getStmtAs(); }
+  void setExpr(Expr *E) { setStmt(E); }
+};
+
 /// This represents 'read' clause in the '#pragma omp atomic' directive.
 ///
 /// \code
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index dcf5dbf449f8b..73ece1e72bbf1 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -3480,6 +3480,11 @@ bool 
RecursiveASTVisitor::VisitOMPAcqRelClause(OMPAcqRelClause *) {
   return true;
 }
 
+template 
+bool RecursiveASTVisitor::VisitOMPHoldsClause(OMPHoldsClause *) {
+  return true;
+}
+
 template 
 bool RecursiveASTVisitor::VisitOMPAcquireClause(OMPAcquireClause *) {
   return true;
diff --git a/clang/include/clang/Sema/SemaOpenMP.h 
b/clang/include/clang/Sema/SemaOpenMP.h
index aa61dae9415e2..7c793cf3077a6 100644
--- a/clang/include/clang/Sema/SemaOpenMP.h
+++ b/clang/include/clang/Sema/SemaOpenMP.h
@@ -940,6 +940,10 @@ class SemaOpenMP : public SemaBase {
  SourceLocation StartLoc,
  SourceLocation LParenLoc,
  SourceLocation EndLoc);
+  /// Called on well-formed 'holds' clause.
+  OMPClause *ActOnOpenMPHoldsClause(Expr *E, SourceLocation StartLoc,
+SourceLocation LParenLoc,
+SourceLocation EndLoc);
 
   OMPClause *ActOnOpenMPSingleExprWithArgClause(
   OpenMPClauseKind Kind, ArrayRef Arguments, Expr *Expr,
diff --git a/clang/lib/AST/OpenMPClause.cpp b/clang/lib/AST/OpenMPClause.cpp
index 042a5df5906ca..e83c81136c34b 100644
--- a/clang/lib/AST/OpenMPClause.cpp
+++ b/clang/lib/AST/OpenMPClause.cpp
@@ -1937,6 +1937,12 @@ void OMPClausePrinter::VisitOMPFailClause(OMPFailClause 
*Node) {
   }
 }
 
+void OMPClausePrinter::VisitOMPHoldsClause(OMPHoldsClause *Node) {
+  OS << "holds(";
+  Node->getExpr()->printPretty(OS, nullptr, Policy, 0);
+  OS << ")";
+}
+
 void OMPClausePrinter::VisitOMPSeqCstClause(OMPSeqCstClause *) {
   OS << "seq_cst";
 }
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index f1e723b4242ee..97e4b3749f94c 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -584,6 +584,8 @@ void OMPClauseProfiler::VisitOMPCompareClause(const 
OMPCompareClause *) {}
 
 void OMPClauseProfiler::VisitOMPFailClause(const OMPFailClause *) {}
 
+void OMPClauseProfiler::VisitOMPHoldsClause(const OMPHoldsClause *) {}
+
 void OMPClauseProfiler::VisitOMPSeqCstClause(const OMPSeqCstClause *) {}
 
 void OMPClauseProfiler::VisitOMPAcqRelClause(const OMPAcqRelClause *) {}
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index e975e96c5c7e4..afcbc93696de2 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -3206,6 +3206,9

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-02 Thread Julian Brown via cfe-commits


jtb20 wrote:

ping?

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-02 Thread Julian Brown via cfe-commits


jtb20 wrote:

Thank you for the review! Again, I do not have commit access, so please apply 
for me if possible.

Julian

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-05 Thread Julian Brown via cfe-commits


jtb20 wrote:

Hopefully fixed by: https://github.com/llvm/llvm-project/pull/102008

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-06 Thread Julian Brown via cfe-commits


jtb20 wrote:

On 06/08/2024 19:40, Valentin Clement (バレンタイン クレメン) wrote:
>   
> This is still breaking a buildbot.
> https://lab.llvm.org/buildbot/#/builders/157/builds/4246
> 
> 
> Are you working on a fix?

It seems to be the same bug -- I assumed the buildbot picked up a
pre-fixed revision.  Is that not the case?

Thanks,

Julian


https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-08-06 Thread Julian Brown via cfe-commits


jtb20 wrote:

Thank you for the update!

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

ping?

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][OpenMP] Diagnose badly-formed collapsed imperfect loop nests (#60678) (PR #101305)

2024-08-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

Ping -- could this patch be applied please?

https://github.com/llvm/llvm-project/pull/101305
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits

jtb20 wrote:

> > FWIW, I don't think there's a reasonable, safe way to collapse loops like 
> > this and maintain parallel semantics, but ICBW.
> 
> I think, there is a proper way. Just annotate these instructions with 
> something like:
> 
> ```
> if (first inner loop iteration) {
>   arr[i][i] = ...;
> }
> ```
> 
> and it should be correct

I think you're right for this particular test case, but I don't think that 
solution works in general. Consider something like:

```
int arr1[N][N], arr2[N];

#pragma omp parallel for collapse(2)
{
  for (int i = 0; i < N; i++) {
arr2[i] = i;
for (int j = 0; j < N; j++) {
  arr1[i][j] += arr2[i];
}
  }
}

```
Now, if collapsed loop iterations take place in parallel or in an arbitrary 
order, there could be a read from arr2[i] before the write (in the "first" 
inner loop iteration). That is, I think we'd have something like:

```
for (int ij = 0; ij < N*N; ij++) {
  int i = ij / N, j = ij % N;
  if (j == 0) {
arr2[i] = i;
  }
  arr1[i][j] += arr2[i];
}
```

and that's still not safe.

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits

jtb20 wrote:

> something like
> 
> ```
> bool Init[N] = {false};
> for (int ij = 0; ij < N*N; ij++) {
>   int i = ij / N, j = ij % N;
>   #pragma omp critical
>   if (!Init[i]) {
> arr2[i] = i;
> Init[i] = true;
>   }
>   arr1[i][j] += arr2[i];
> }
> ```

I don't think synthesizing a critical region inside the collapsed loop will be 
a win overall, and also I don't quite see how it helps in this case. A correct 
version would be doing something like serialising the loop, I think, which I 
suppose sort of bypasses the problem, but doesn't really solve it. (When would 
we do such a transformation? How much analysis do we need to avoid killing 
performance in non-contrived cases?)

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits

jtb20 wrote:

> > I don't think synthesizing a critical region inside the collapsed loop will 
> > be a win overall, and also I don't quite see how it helps in this case. A 
> > correct version would be doing something like serialising the loop, I 
> > think, which I suppose sort of bypasses the problem, but doesn't really 
> > solve it. (When would we do such a transformation? How much analysis do we 
> > need to avoid killing performance in non-contrived cases?)
> 
> It should be done in the frontend. And here we should care about correctness, 
> not performance. Sure, this can be optimized for better performance later in 
> OpenMPOpt pass, but from the frontend we should get it working correctly.

Apologies -- I meant "when would we do such a transformation" as "under what 
precise conditions would we do it" rather than "at what point in the 
compilation pipeline".

I think you're suggesting that we should only perform a genuine loop collapse 
if we can prove that it is safe (else falling back to some other code 
transformation*). IIUC that sort of thing isn't normally done with OpenMP -- 
the compiler does what the user tells it to. Any parallel loop with 
inter-iteration dependencies will probably compile to wrong code. It's just 
that for this particular case it seems especially easy to shoot oneself in the 
foot.

(*) Indeed, maybe just backing out of collapsing the loop nest altogether might 
be the quickest/safest option. Doing that conservatively might mean essentially 
never collapsing imperfectly-nested loops though. Is that what we want?

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

Can you explain why the critical section version of the code is safe, and why 
it is an improvement over simply not collapsing the loop?

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

2024-08-19 Thread Julian Brown via cfe-commits

jtb20 wrote:

> > Can you explain why the critical section version of the code is safe, and 
> > why it is an improvement over simply not collapsing the loop?
> 
> 1. I'm not saying it is safe, I'm just saying that something like this might 
> be safe
> 2. Without properly collapsing the loops, the compiler won't be able to 
> properly schedule the execution per programmer's request

Yes, OK.

The trouble is, I'm still not sure that there **is** "something like this" that 
lets the collapsed loop execute correctly with independent iterations. (There 
might be declarations in the "imperfect" part of the loop -- that's probably 
the most likely thing to be there, in fact. That makes conditionalising those 
bits or putting them inside other synthesised directives more complicated). 
Maybe I'll think about it some more.

https://github.com/llvm/llvm-project/pull/96087
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] SemaOpenMP.cpp and StmtOpenMP.cpp spelling fixes (PR #96814)

2024-06-26 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created https://github.com/llvm/llvm-project/pull/96814

This patch just fixes a few spelling mistakes in the above two files. (I 
changed one British spelling to American -- analyse to analyze -- because the 
latter spelling is used elsewhere in file, and it's probably best to be 
consistent.)

>From a1042898cdb864d2a867753b5ea57809fe28f01e Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 26 Jun 2024 11:17:56 -0500
Subject: [PATCH] [OpenMP] SemaOpenMP.cpp and StmtOpenMP.cpp spelling fixes

This patch just fixes a few spelling mistakes in the above two files.
(I changed one British spelling to American -- analyse to analyze --
because the latter spelling is used elsewhere in file, and it's probably
best to be consistent.)
---
 .../clang/Basic/DiagnosticSemaKinds.td|  2 +-
 clang/lib/AST/StmtOpenMP.cpp  |  2 +-
 clang/lib/Sema/SemaOpenMP.cpp | 44 +--
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 79cc9c61f7fd3..b9922231be2c5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -0,7 +0,7 @@ def err_omp_loop_variable_type : Error<
 def err_omp_loop_incr_not_compatible : Error<
   "increment expression must cause %0 to %select{decrease|increase}1 "
   "on each iteration of OpenMP for loop">;
-def note_omp_loop_cond_requres_compatible_incr : Note<
+def note_omp_loop_cond_requires_compatible_incr : Note<
   "loop step is expected to be %select{negative|positive}0 due to this 
condition">;
 def err_omp_loop_diff_cxx : Error<
   "could not calculate number of iterations calling 'operator-' with "
diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index d8519b2071e6d..c8792941a6bb6 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 //
-// This file implements the subclesses of Stmt class declared in StmtOpenMP.h
+// This file implements the subclasses of Stmt class declared in StmtOpenMP.h
 //
 
//===--===//
 
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 7697246ea5e59..3d14ce920af31 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -466,7 +466,7 @@ class DSAStackTy {
 getTopOfStack().PossiblyLoopCounter = D ? D->getCanonicalDecl() : D;
   }
   /// Gets the possible loop counter decl.
-  const Decl *getPossiblyLoopCunter() const {
+  const Decl *getPossiblyLoopCounter() const {
 return getTopOfStack().PossiblyLoopCounter;
   }
   /// Start new OpenMP region stack in new non-capturing function.
@@ -718,7 +718,7 @@ class DSAStackTy {
 TargetLocations.push_back(LocStart);
   }
 
-  /// Add location for the first encountered atomicc directive.
+  /// Add location for the first encountered atomic directive.
   void addAtomicDirectiveLoc(SourceLocation Loc) {
 if (AtomicLocation.isInvalid())
   AtomicLocation = Loc;
@@ -2584,7 +2584,7 @@ OpenMPClauseKind 
SemaOpenMP::isOpenMPPrivateDecl(ValueDecl *D, unsigned Level,
   DSAStack->loopStart();
   return OMPC_private;
 }
-if ((DSAStack->getPossiblyLoopCunter() == D->getCanonicalDecl() ||
+if ((DSAStack->getPossiblyLoopCounter() == D->getCanonicalDecl() ||
  DSAStack->isLoopControlVariable(D).first) &&
 !DSAStack->hasExplicitDSA(
 D, [](OpenMPClauseKind K, bool) { return K != OMPC_private; },
@@ -2694,8 +2694,8 @@ bool SemaOpenMP::isOpenMPGlobalCapturedDecl(ValueDecl *D, 
unsigned Level,
   unsigned NumLevels =
   getOpenMPCaptureLevels(DSAStack->getDirective(Level));
   if (Level == 0)
-// non-file scope static variale with default(firstprivate)
-// should be gloabal captured.
+// non-file scope static variable with default(firstprivate)
+// should be global captured.
 return (NumLevels == CaptureLevel + 1 &&
 (TopDVar.CKind != OMPC_shared ||
  DSAStack->getDefaultDSA() == DSA_firstprivate));
@@ -2730,11 +2730,11 @@ void SemaOpenMP::finalizeOpenMPDelayedAnalysis(const 
FunctionDecl *Caller,
   assert(getLangOpts().OpenMP && "Expected OpenMP compilation mode.");
   std::optional DevTy =
   OMPDeclareTargetDeclAttr::getDeviceType(Caller->getMostRecentDecl());
-  // Ignore host functions during device analyzis.
+  // Ignore host functions during device analysis.
   if (getLangOpts().OpenMPIsTargetDevice &&
   (!DevTy || *DevTy == OMPDeclareTargetDeclAttr::DT_Host))
 return;
-  // Ignore nohost functions during host analyzis.
+  // Ignore nohost functions during host analysis.
   if (!getLangOpts().OpenMPIsTargetDevice && DevTy &&
   *DevTy ==

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-26 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 65db444a6faf8d246e286e04675f2b0a06909723 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 21 
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 00f9ebb65d876..200185a63aaa6 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2920,6 +2920,27 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  EndLoc = Tok.getLocation();
+  Directive = Actions.ActOnCompoundStmt(Loc, EndLoc,
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 7697246ea5e59..e461a950cdc7f 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..a38380ed4482a
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
\ No newline at end of file
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 100644
index 0..33c1c6f7c51e7
--- /dev/null
+++ b/clang/test/OpenMP/assume_messages.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -verify -fopenmp -x c -std=c99 %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -verify -fopenmp-simd -x c 
-std=c99 %s
+
+#pragma omp assume no_openmp // expected-error {{unexpected OpenMP directive 
'#pragma

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-27 Thread Julian Brown via cfe-commits



@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}

jtb20 wrote:

Understood! There is indeed a vscode option to add the missing newline (it 
appears to be turned off by default for some bizarre reason). I'll push a new 
version with them in.

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-27 Thread Julian Brown via cfe-commits

jtb20 wrote:

> > > don't you need more code in AST?
> > 
> > 
> > Sorry, I don't quite understand the question! Could you elaborate a little 
> > please?
> 
> I was thinking maybe you need changes in AST related files, like 
> `ASTWriter.cpp`, but that might be not needed as this is adding a new 
> directive.

At the moment, since the "assume" directive is parsed but then immediately 
discarded, I don't think anything else is needed.

Actually the existing "assumes" support is reused -- the bit in SemaOpenMP.cpp 
adds the "assume" assumptions to the OMPAssumeScoped "stack". For "assumes", 
it's done like that so (e.g. top-level) declarations between begin/end 
"assumes" can be modified according to the assumptions on that stack. For 
"assume", once some use is made for the assumptions, that might turn out to not 
be the most useful representation. That can be revisited later though, I think.

https://github.com/llvm/llvm-project/pull/92731
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [OpenMP] OpenMP 5.1 "assume" directive parsing support (PR #92731)

2024-06-27 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated https://github.com/llvm/llvm-project/pull/92731

>From 9107c1a0b215cc9af17f0168961b59edca3a9127 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Wed, 1 May 2024 06:35:59 -0500
Subject: [PATCH] [OpenMP] OpenMP 5.1 "assume" directive parsing support

This is a minimal patch to support parsing for "omp assume" directives.
These are meant to be hints to a compiler' optimisers: as such, it is
legitimate (if not very useful) to ignore them.  The patch builds on top
of the existing support for "omp assumes" directives (note spelling!).

Unlike the "omp [begin/end] assumes" directives, "omp assume" is
associated with a compound statement, i.e. it can appear within a
function.  The "holds" assumption could (theoretically) be mapped onto
the existing builtin "__builtin_assume", though the latter applies to a
single point in the program, and the former to a range (i.e. the whole
of the associated compound statement).

This patch fixes sollve's OpenMP 5.1 "omp assume"-based tests.

Change-Id: Ibd4a0e2af82c4ac818eaa3de8867a006307361ec
---
 clang/lib/Parse/ParseOpenMP.cpp  | 25 ++
 clang/lib/Sema/SemaOpenMP.cpp|  3 +-
 clang/test/OpenMP/assume_lambda.cpp  | 31 +
 clang/test/OpenMP/assume_messages.c  | 23 +
 clang/test/OpenMP/assume_messages_attr.c | 23 +
 clang/test/OpenMP/assume_template.cpp| 42 
 llvm/include/llvm/Frontend/OpenMP/OMP.td |  4 +++
 7 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/OpenMP/assume_lambda.cpp
 create mode 100644 clang/test/OpenMP/assume_messages.c
 create mode 100644 clang/test/OpenMP/assume_messages_attr.c
 create mode 100644 clang/test/OpenMP/assume_template.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 00f9ebb65d876..4b378bc83b7d1 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2371,6 +2371,11 @@ Parser::DeclGroupPtrTy 
Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
 ParseOMPEndDeclareTargetDirective(DTCI.Kind, DKind, DTCI.Loc);
 return nullptr;
   }
+  case OMPD_assume: {
+Diag(Tok, diag::err_omp_unexpected_directive)
+<< 1 << getOpenMPDirectiveName(DKind);
+break;
+  }
   case OMPD_unknown:
 Diag(Tok, diag::err_omp_unknown_directive);
 break;
@@ -2920,6 +2925,26 @@ StmtResult 
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
 << 1 << getOpenMPDirectiveName(DKind);
 SkipUntil(tok::annot_pragma_openmp_end);
 break;
+  case OMPD_assume: {
+ParseScope OMPDirectiveScope(this, Scope::FnScope | Scope::DeclScope |
+ Scope::CompoundStmtScope);
+ParseOpenMPAssumesDirective(DKind, ConsumeToken());
+
+SkipUntil(tok::annot_pragma_openmp_end);
+
+ParsingOpenMPDirectiveRAII NormalScope(*this);
+StmtResult AssociatedStmt;
+{
+  Sema::CompoundScopeRAII Scope(Actions);
+  AssociatedStmt = ParseStatement();
+  Directive = Actions.ActOnCompoundStmt(Loc, Tok.getLocation(),
+AssociatedStmt.get(),
+/*isStmtExpr=*/false);
+}
+ParseOpenMPEndAssumesDirective(Loc);
+OMPDirectiveScope.Exit();
+break;
+  }
   case OMPD_unknown:
   default:
 Diag(Tok, diag::err_omp_unknown_directive);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 7697246ea5e59..e461a950cdc7f 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3512,7 +3512,8 @@ void 
SemaOpenMP::ActOnOpenMPAssumesDirective(SourceLocation Loc,
 
   auto *AA =
   OMPAssumeAttr::Create(getASTContext(), llvm::join(Assumptions, ","), 
Loc);
-  if (DKind == llvm::omp::Directive::OMPD_begin_assumes) {
+  if (DKind == llvm::omp::Directive::OMPD_begin_assumes ||
+  DKind == llvm::omp::Directive::OMPD_assume) {
 OMPAssumeScoped.push_back(AA);
 return;
   }
diff --git a/clang/test/OpenMP/assume_lambda.cpp 
b/clang/test/OpenMP/assume_lambda.cpp
new file mode 100644
index 0..b1ab71617478e
--- /dev/null
+++ b/clang/test/OpenMP/assume_lambda.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -ast-print %s | FileCheck %s
+// expected-no-diagnostics
+
+extern int bar(int);
+
+int foo(int arg)
+{
+  #pragma omp assume no_openmp_routines
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
+
+class C {
+public:
+  int foo(int a);
+};
+
+// We're really just checking that this parses.  All the assumptions are thrown
+// away immediately for now.
+int C::foo(int a)
+{
+  #pragma omp assume holds(sizeof(T) == 8) absent(parallel)
+  {
+auto fn = [](int x) { return bar(x); };
+// CHECK: auto fn = [](int x) {
+return fn(5);
+  }
+}
diff --git a/clang/test/OpenMP/assume_messages.c 
b/clang/test/OpenMP/assume_messages.c
new file mode 1006

[clang] [llvm] [Clang][OpenMP] Support for dispatch construct (Sema & Codegen) support (PR #117904)

2025-02-12 Thread Julian Brown via cfe-commits


jtb20 wrote:

The failures with this patch and not mine are just:

PASS->FAIL: tests/5.1/dispatch/test_dispatch_is_device_ptr.c run
PASS->FAIL: tests/5.1/dispatch/test_dispatch_nowait.c run

that's actually probably neither here nor there, the clauses in question aren't 
implemented in my patch yet so the tests are likely succeeding "by accident".

https://github.com/llvm/llvm-project/pull/117904
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Mark 'map-type modifiers in arbitrary position' done (PR #133906)

2025-04-04 Thread Julian Brown via cfe-commits


https://github.com/jtb20 closed https://github.com/llvm/llvm-project/pull/133906
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits



@@ -7080,6 +7080,111 @@ class MappableExprsHandler {
 return ConstLength.getSExtValue() != 1;
   }
 
+  /// A helper class to copy structures with overlapped elements, i.e. those
+  /// which have mappings of both "s" and "s.mem".  Consecutive elements that
+  /// are not explicitly copied have mapping nodes synthesized for them,
+  /// taking care to avoid generating zero-sized copies.
+  class CopyOverlappedEntryGaps {
+CodeGenFunction &CGF;
+MapCombinedInfoTy &CombinedInfo;
+OpenMPOffloadMappingFlags Flags;
+const ValueDecl *MapDecl;
+const Expr *MapExpr;
+Address BP;
+bool IsNonContiguous;
+uint64_t DimSize;
+// These elements track the position as the struct is iterated over
+// (in order of increasing element address).
+const RecordDecl *LastParent = nullptr;
+uint64_t Cursor = 0;
+unsigned LastIndex = -1u;
+Address LB;
+
+  public:
+CopyOverlappedEntryGaps(CodeGenFunction &_CGF,
+MapCombinedInfoTy &_CombinedInfo,
+OpenMPOffloadMappingFlags _Flags,
+const ValueDecl *_MapDecl, const Expr *_MapExpr,
+Address _BP, Address _LB, bool _IsNonContiguous,

jtb20 wrote:

Fixed, thanks.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits



@@ -7080,6 +7080,111 @@ class MappableExprsHandler {
 return ConstLength.getSExtValue() != 1;
   }
 
+  /// A helper class to copy structures with overlapped elements, i.e. those
+  /// which have mappings of both "s" and "s.mem".  Consecutive elements that
+  /// are not explicitly copied have mapping nodes synthesized for them,
+  /// taking care to avoid generating zero-sized copies.
+  class CopyOverlappedEntryGaps {
+CodeGenFunction &CGF;
+MapCombinedInfoTy &CombinedInfo;
+OpenMPOffloadMappingFlags Flags;
+const ValueDecl *MapDecl;
+const Expr *MapExpr;
+Address BP;
+bool IsNonContiguous;
+uint64_t DimSize;
+// These elements track the position as the struct is iterated over
+// (in order of increasing element address).
+const RecordDecl *LastParent = nullptr;
+uint64_t Cursor = 0;
+unsigned LastIndex = -1u;
+Address LB;
+
+  public:
+CopyOverlappedEntryGaps(CodeGenFunction &_CGF,
+MapCombinedInfoTy &_CombinedInfo,
+OpenMPOffloadMappingFlags _Flags,
+const ValueDecl *_MapDecl, const Expr *_MapExpr,
+Address _BP, Address _LB, bool _IsNonContiguous,
+uint64_t _DimSize)
+: CGF(_CGF), CombinedInfo(_CombinedInfo), Flags(_Flags),
+  MapDecl(_MapDecl), MapExpr(_MapExpr), BP(_BP), LB(_LB),
+  IsNonContiguous(_IsNonContiguous), DimSize(_DimSize) {}
+
+void ProcessField(
+const OMPClauseMappableExprCommon::MappableComponent &MC,
+const FieldDecl *FD,
+llvm::function_ref
+EmitMemberExprBase) {
+  const RecordDecl *RD = FD->getParent();
+  const ASTRecordLayout &RL = CGF.getContext().getASTRecordLayout(RD);
+  uint64_t FieldOffset = RL.getFieldOffset(FD->getFieldIndex());
+  uint64_t FieldSize =
+  CGF.getContext().getTypeSize(FD->getType().getCanonicalType());
+  Address ComponentLB = Address::invalid();
+
+  if (FD->getType()->isLValueReferenceType()) {
+const auto *ME = cast(MC.getAssociatedExpression());
+LValue BaseLVal = EmitMemberExprBase(CGF, ME);
+ComponentLB =
+CGF.EmitLValueForFieldInitialization(BaseLVal, FD).getAddress();
+  } else {
+ComponentLB =
+CGF.EmitOMPSharedLValue(MC.getAssociatedExpression()).getAddress();
+  }
+
+  if (LastParent == nullptr) {
+LastParent = RD;
+  }
+  if (FD->getParent() == LastParent) {
+if (FD->getFieldIndex() != LastIndex + 1)
+  CopyUntilField(FD, ComponentLB);
+  } else {
+LastParent = FD->getParent();
+if (((int64_t)FieldOffset - (int64_t)Cursor) > 0)
+  CopyUntilField(FD, ComponentLB);
+  }
+  Cursor = FieldOffset + FieldSize;
+  LastIndex = FD->getFieldIndex();
+  LB = CGF.Builder.CreateConstGEP(ComponentLB, 1);
+}
+
+void CopyUntilField(const FieldDecl *FD, Address ComponentLB) {
+  llvm::Value *ComponentLBPtr = ComponentLB.emitRawPointer(CGF);
+  llvm::Value *LBPtr = LB.emitRawPointer(CGF);
+  llvm::Value *Size =
+  CGF.Builder.CreatePtrDiff(CGF.Int8Ty, ComponentLBPtr, LBPtr);
+  CopySizedChunk(LBPtr, Size);
+}
+
+void CopyUntilEnd(Address HB) {
+  if (LastParent) {
+const ASTRecordLayout &RL =
+CGF.getContext().getASTRecordLayout(LastParent);
+if ((uint64_t)CGF.getContext().toBits(RL.getSize()) <= Cursor)
+  return;
+  }
+  llvm::Value *LBPtr = LB.emitRawPointer(CGF);
+  llvm::Value *Size = CGF.Builder.CreatePtrDiff(
+  CGF.Int8Ty, CGF.Builder.CreateConstGEP(HB, 1).emitRawPointer(CGF),
+  LBPtr);
+  CopySizedChunk(LBPtr, Size);
+}
+
+void CopySizedChunk(llvm::Value *Base, llvm::Value *Size) {

jtb20 wrote:

Fixed, thanks.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits



@@ -7080,6 +7080,111 @@ class MappableExprsHandler {
 return ConstLength.getSExtValue() != 1;
   }
 
+  /// A helper class to copy structures with overlapped elements, i.e. those
+  /// which have mappings of both "s" and "s.mem".  Consecutive elements that
+  /// are not explicitly copied have mapping nodes synthesized for them,
+  /// taking care to avoid generating zero-sized copies.
+  class CopyOverlappedEntryGaps {
+CodeGenFunction &CGF;
+MapCombinedInfoTy &CombinedInfo;
+OpenMPOffloadMappingFlags Flags;
+const ValueDecl *MapDecl;
+const Expr *MapExpr;
+Address BP;
+bool IsNonContiguous;
+uint64_t DimSize;
+// These elements track the position as the struct is iterated over
+// (in order of increasing element address).
+const RecordDecl *LastParent = nullptr;
+uint64_t Cursor = 0;
+unsigned LastIndex = -1u;
+Address LB;
+
+  public:
+CopyOverlappedEntryGaps(CodeGenFunction &_CGF,
+MapCombinedInfoTy &_CombinedInfo,
+OpenMPOffloadMappingFlags _Flags,
+const ValueDecl *_MapDecl, const Expr *_MapExpr,
+Address _BP, Address _LB, bool _IsNonContiguous,
+uint64_t _DimSize)
+: CGF(_CGF), CombinedInfo(_CombinedInfo), Flags(_Flags),
+  MapDecl(_MapDecl), MapExpr(_MapExpr), BP(_BP), LB(_LB),
+  IsNonContiguous(_IsNonContiguous), DimSize(_DimSize) {}
+
+void ProcessField(
+const OMPClauseMappableExprCommon::MappableComponent &MC,
+const FieldDecl *FD,
+llvm::function_ref
+EmitMemberExprBase) {
+  const RecordDecl *RD = FD->getParent();
+  const ASTRecordLayout &RL = CGF.getContext().getASTRecordLayout(RD);
+  uint64_t FieldOffset = RL.getFieldOffset(FD->getFieldIndex());
+  uint64_t FieldSize =
+  CGF.getContext().getTypeSize(FD->getType().getCanonicalType());
+  Address ComponentLB = Address::invalid();
+
+  if (FD->getType()->isLValueReferenceType()) {
+const auto *ME = cast(MC.getAssociatedExpression());
+LValue BaseLVal = EmitMemberExprBase(CGF, ME);
+ComponentLB =
+CGF.EmitLValueForFieldInitialization(BaseLVal, FD).getAddress();
+  } else {
+ComponentLB =
+CGF.EmitOMPSharedLValue(MC.getAssociatedExpression()).getAddress();
+  }
+
+  if (LastParent == nullptr) {
+LastParent = RD;
+  }

jtb20 wrote:

Changed, thanks.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits



@@ -7080,6 +7080,111 @@ class MappableExprsHandler {
 return ConstLength.getSExtValue() != 1;
   }
 
+  /// A helper class to copy structures with overlapped elements, i.e. those
+  /// which have mappings of both "s" and "s.mem".  Consecutive elements that
+  /// are not explicitly copied have mapping nodes synthesized for them,
+  /// taking care to avoid generating zero-sized copies.
+  class CopyOverlappedEntryGaps {
+CodeGenFunction &CGF;
+MapCombinedInfoTy &CombinedInfo;
+OpenMPOffloadMappingFlags Flags;
+const ValueDecl *MapDecl;
+const Expr *MapExpr;
+Address BP;
+bool IsNonContiguous;
+uint64_t DimSize;
+// These elements track the position as the struct is iterated over
+// (in order of increasing element address).
+const RecordDecl *LastParent = nullptr;
+uint64_t Cursor = 0;
+unsigned LastIndex = -1u;
+Address LB;

jtb20 wrote:

Done.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/148947

>From c30dcc39a2b14eb0aef51e5f8bb34b7338e587d0 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Thu, 19 Jun 2025 07:47:38 -0500
Subject: [PATCH] [OpenMP] Don't emit redundant zero-sized mapping nodes for
 overlapped structs

The handling of overlapped structure mapping in CGOpenMPRuntime.cpp can
lead to redundant zero-sized mapping nodes at runtime.  This patch fixes
it using a combination of approaches: trivially adjacent struct members
won't have a mapping node created between them, and for more complicated
cases (inheritance) the physical layout of the struct/class is used to
make sure that elements aren't missed.

I've introduced a new class to track the state whilst iterating over
the struct.  This reduces a bit of redundancy in the code (accumulating
CombinedInfo both during and after the loop), which I think is a bit
neater.

Before:

omptarget --> Entry  0: Base=0x7fff8d483830, Begin=0x7fff8d483830, 
Size=48, Type=0x20, Name=unknown
omptarget --> Entry  1: Base=0x7fff8d483830, Begin=0x7fff8d483830, 
Size=0, Type=0x10003, Name=unknown
omptarget --> Entry  2: Base=0x7fff8d483830, Begin=0x7fff8d483834, 
Size=0, Type=0x10003, Name=unknown
omptarget --> Entry  3: Base=0x7fff8d483830, Begin=0x7fff8d483838, 
Size=0, Type=0x10003, Name=unknown
omptarget --> Entry  4: Base=0x7fff8d483830, Begin=0x7fff8d48383c, 
Size=20, Type=0x10003, Name=unknown
omptarget --> Entry  5: Base=0x7fff8d483830, Begin=0x7fff8d483854, 
Size=0, Type=0x10003, Name=unknown
omptarget --> Entry  6: Base=0x7fff8d483830, Begin=0x7fff8d483858, 
Size=0, Type=0x10003, Name=unknown
omptarget --> Entry  7: Base=0x7fff8d483830, Begin=0x7fff8d48385c, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  8: Base=0x7fff8d483830, Begin=0x7fff8d483830, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  9: Base=0x7fff8d483830, Begin=0x7fff8d483834, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry 10: Base=0x7fff8d483830, Begin=0x7fff8d483838, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry 11: Base=0x7fff8d483840, Begin=0x5e7665275130, 
Size=32, Type=0x10013, Name=unknown
omptarget --> Entry 12: Base=0x7fff8d483830, Begin=0x7fff8d483850, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry 13: Base=0x7fff8d483830, Begin=0x7fff8d483854, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry 14: Base=0x7fff8d483830, Begin=0x7fff8d483858, 
Size=4, Type=0x10003, Name=unknown

After:

omptarget --> Entry  0: Base=0x7fffd0f562e0, Begin=0x7fffd0f562e0, 
Size=48, Type=0x20, Name=unknown
omptarget --> Entry  1: Base=0x7fffd0f562e0, Begin=0x7fffd0f562ec, 
Size=20, Type=0x10003, Name=unknown
omptarget --> Entry  2: Base=0x7fffd0f562e0, Begin=0x7fffd0f5630c, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  3: Base=0x7fffd0f562e0, Begin=0x7fffd0f562e0, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  4: Base=0x7fffd0f562e0, Begin=0x7fffd0f562e4, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  5: Base=0x7fffd0f562e0, Begin=0x7fffd0f562e8, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  6: Base=0x7fffd0f562f0, Begin=0x58b6013fb130, 
Size=32, Type=0x10013, Name=unknown
omptarget --> Entry  7: Base=0x7fffd0f562e0, Begin=0x7fffd0f56300, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  8: Base=0x7fffd0f562e0, Begin=0x7fffd0f56304, 
Size=4, Type=0x10003, Name=unknown
omptarget --> Entry  9: Base=0x7fffd0f562e0, Begin=0x7fffd0f56308, 
Size=4, Type=0x10003, Name=unknown

For code:

  #include 
  #include 

  struct S {
int x;
int y;
int z;
int *p1;
int *p2;
  };

  struct T : public S {
int a;
int b;
int c;
  };

  int main() {
T v;
v.p1 = (int*) calloc(8, sizeof(int));
v.p2 = (int*) calloc(8, sizeof(int));

  #pragma omp target map(tofrom: v, v.x, v.y, v.z, v.p1[:8], v.a, v.b, v.c)
{
  v.x++;
  v.y += 2;
  v.z += 3;
  v.p1[0] += 4;
  v.a += 7;
  v.b += 5;
  v.c += 6;
}

return 0;
  }
---
 clang/lib/CodeGen/CGOpenMPRuntime.cpp   | 157 ++--
 clang/test/OpenMP/copy-gaps-1.cpp   |  52 +++
 clang/test/OpenMP/copy-gaps-2.cpp   |  52 +++
 clang/test/OpenMP/copy-gaps-3.cpp   |  46 ++
 clang/test/OpenMP/copy-gaps-4.cpp   |  48 ++
 clang/test/OpenMP/copy-gaps-5.cpp   |  50 +++
 clang/test/OpenMP/copy-gaps-6.cpp   |  87 +++
 clang/test/OpenMP/target_map_codegen_35.cpp |  29 +---
 8 files changed, 449 insertions(+), 72 deletions(-)
 create mode 100644 clang/test/OpenMP/copy

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-16 Thread Julian Brown via cfe-commits



@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu 
-fopenmp-targets=amdgcn-amd-amdhsa -fopenmp -emit-llvm %s -o - | FileCheck %s

jtb20 wrote:

This one though I'm not sure how to do -- the check lines for the new copy-gaps 
tests are manually written, as it appears were the check lines for the altered 
test target_map_codegen_35.cpp (which seems to be the only extant test that 
this patch affects). Would it be better to have auto-generated check lines for 
the whole generated output for the new tests? That seems like it'd be more 
brittle, to me.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-24 Thread Julian Brown via cfe-commits


jtb20 wrote:

I'll have a look.

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Fix initialization order for CopyOverlappedEntryGaps (PR #150431)

2025-07-24 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created 
https://github.com/llvm/llvm-project/pull/150431

NFC.

>From 806a34cf4ef936f22d0738f9bf34446b54d9fa21 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Thu, 24 Jul 2025 09:35:36 -0500
Subject: [PATCH] [OpenMP] Fix initialization order for CopyOverlappedEntryGaps

NFC.
---
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f1698a0bec373..91237cfe3a372 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7108,8 +7108,8 @@ class MappableExprsHandler {
 Address BP, Address LB, bool IsNonContiguous,
 uint64_t DimSize)
 : CGF(CGF), CombinedInfo(CombinedInfo), Flags(Flags), MapDecl(MapDecl),
-  MapExpr(MapExpr), BP(BP), LB(LB), IsNonContiguous(IsNonContiguous),
-  DimSize(DimSize) {}
+  MapExpr(MapExpr), BP(BP), IsNonContiguous(IsNonContiguous),
+  DimSize(DimSize), LB(LB) {}
 
 void processField(
 const OMPClauseMappableExprCommon::MappableComponent &MC,

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Don't emit redundant zero-sized mapping nodes for overlapped structs (PR #148947)

2025-07-24 Thread Julian Brown via cfe-commits


jtb20 wrote:

NFC fix (hopefully!): https://github.com/llvm/llvm-project/pull/150431

https://github.com/llvm/llvm-project/pull/148947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP][wip] Rework 'containing struct'/overlapped mapping handling (PR #153672)

2025-08-14 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created 
https://github.com/llvm/llvm-project/pull/153672

This patch is an early outline for a rework of several mapping features, 
intended to support 'containing structures' in the middle of expressions as 
well as at the top level (https://github.com/llvm/llvm-project/issues/141042).  
Key ideas are:

  - struct information is gathered using several pre-passes before 
`generateInfoForComponentList`.

  - "PartialStruct" is turned into a map, keyed on the "effective base" of each 
containing structure in a set of expressions in OpenMP 'map' clauses.

  - the reverse iterator over component lists (visiting the base decl, then 
walking out to the full expression) has a new 'ComponentListRefPtrPteeIterator' 
adapter that (a) visits reference-type list components twice, and (b) provides 
a few useful utility methods.

The current state is that a couple of tests work up to a point, but I'm hitting 
problems with the runtime that will probably be helped by the in-progress 
patches to support ATTACH operations.

This is obviously all full of debug code and not at all ready for review! 
Posting FYI.

>From 806a34cf4ef936f22d0738f9bf34446b54d9fa21 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Thu, 24 Jul 2025 09:35:36 -0500
Subject: [PATCH 1/3] [OpenMP] Fix initialization order for
 CopyOverlappedEntryGaps

NFC.
---
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f1698a0bec373..91237cfe3a372 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7108,8 +7108,8 @@ class MappableExprsHandler {
 Address BP, Address LB, bool IsNonContiguous,
 uint64_t DimSize)
 : CGF(CGF), CombinedInfo(CombinedInfo), Flags(Flags), MapDecl(MapDecl),
-  MapExpr(MapExpr), BP(BP), LB(LB), IsNonContiguous(IsNonContiguous),
-  DimSize(DimSize) {}
+  MapExpr(MapExpr), BP(BP), IsNonContiguous(IsNonContiguous),
+  DimSize(DimSize), LB(LB) {}
 
 void processField(
 const OMPClauseMappableExprCommon::MappableComponent &MC,

>From a0b521344ba96cd47b70dca969ae836579c29dfc Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Tue, 10 Jun 2025 08:11:06 -0500
Subject: [PATCH 2/3] Revert "[Clang][OpenMP] Fix mapping of structs to device
 (#75642)"

This reverts commit 4ef6587715bec4520332528c4a71fe5a9ac10477.
---
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 116 +-
 1 file changed, 20 insertions(+), 96 deletions(-)

diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 91237cfe3a372..3bfb01cccf1a0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7193,10 +7193,8 @@ class MappableExprsHandler {
   OpenMPMapClauseKind MapType, ArrayRef 
MapModifiers,
   ArrayRef MotionModifiers,
   OMPClauseMappableExprCommon::MappableExprComponentListRef Components,
-  MapCombinedInfoTy &CombinedInfo,
-  MapCombinedInfoTy &StructBaseCombinedInfo,
-  StructRangeInfoTy &PartialStruct, bool IsFirstComponentList,
-  bool IsImplicit, bool GenerateAllInfoForClauses,
+  MapCombinedInfoTy &CombinedInfo, StructRangeInfoTy &PartialStruct,
+  bool IsFirstComponentList, bool IsImplicit,
   const ValueDecl *Mapper = nullptr, bool ForDeviceAddr = false,
   const ValueDecl *BaseDecl = nullptr, const Expr *MapExpr = nullptr,
   ArrayRef
@@ -7493,25 +7491,6 @@ class MappableExprsHandler {
 bool IsPartialMapped =
 !PartialStruct.PreliminaryMapData.BasePointers.empty();
 
-// We need to check if we will be encountering any MEs. If we do not
-// encounter any ME expression it means we will be mapping the whole 
struct.
-// In that case we need to skip adding an entry for the struct to the
-// CombinedInfo list and instead add an entry to the StructBaseCombinedInfo
-// list only when generating all info for clauses.
-bool IsMappingWholeStruct = true;
-if (!GenerateAllInfoForClauses) {
-  IsMappingWholeStruct = false;
-} else {
-  for (auto TempI = I; TempI != CE; ++TempI) {
-const MemberExpr *PossibleME =
-dyn_cast(TempI->getAssociatedExpression());
-if (PossibleME) {
-  IsMappingWholeStruct = false;
-  break;
-}
-  }
-}
-
 for (; I != CE; ++I) {
   // If the current component is member of a struct (parent struct) mark 
it.
   if (!EncounteredME) {
@@ -7701,7 +7680,6 @@ class MappableExprsHandler {
 // PartialStruct.PreliminaryMapData.BasePointers has been mapped.
 if ((!IsMemberPointerOrAddr && !IsPartialMapped) ||
 (Next == CE && MapType != OMPC_MAP_unknown)) {
-  if (!IsMappingWholeStruct) {
 CombinedInfo.Exprs.emplace_back(MapDecl, MapE

[clang] [OpenMP][wip] Rework 'containing struct'/overlapped mapping handling (PR #153672)

2025-08-14 Thread Julian Brown via cfe-commits


https://github.com/jtb20 converted_to_draft 
https://github.com/llvm/llvm-project/pull/153672
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP][wip] Rework 'containing struct'/overlapped mapping handling (PR #153672)

2025-08-14 Thread Julian Brown via cfe-commits


jtb20 wrote:

@abhinavgaba @alexey-bataev 

https://github.com/llvm/llvm-project/pull/153672
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP][wip] Rework 'containing struct'/overlapped mapping handling (PR #153672)

2025-08-14 Thread Julian Brown via cfe-commits


jtb20 wrote:

Thank you for the reply! I see that your patch series is tackling some of the 
same problems as mine, so I hope there's some useful non-overlapping bits here. 
I'll be able to have a closer look after next week.

https://github.com/llvm/llvm-project/pull/153672
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][OpenMP] Handle check for pointer-based array sections (PR #157443)

2025-09-08 Thread Julian Brown via cfe-commits


jtb20 wrote:

Does this patch work for the test_target_update_mapper_{to,from}_discontiguous 
tests?

https://github.com/llvm/llvm-project/pull/157443
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][OpenMP] Handle check for pointer-based array sections (PR #157443)

2025-09-08 Thread Julian Brown via cfe-commits


jtb20 wrote:

I had a quick look because I was looking into this briefly too -- it still 
crashes for me, like so:
```
Thread 1 "test_target_upd" received signal SIGSEGV, Segmentation fault.
0x77aec887 in targetDataUpdate (Loc=0xb5c8, Device=..., 
ArgNum=2, ArgsBase=0x7fffd880, Args=0x7fffd870, 
ArgSizes=0x7fffd860, ArgTypes=0x82d0, ArgNames=0xb5b8, 
ArgMappers=0x0, AsyncInfo=..., AttachInfo=0x0, FromMapper=false) at 
/work1/julbrown/code/mainline+amd-staging/src/llvm-project-main/offload/libomptarget/omptarget.cpp:1297
1297  NonContig[DimSize - 1].Count * NonContig[DimSize - 1].Stride;
```
The strange thing about the surrounding loop (in 
`generateInfoForComponentList`) is that it *appears* like it was written to 
handle this case at some point, see e.g. the comment:
```
// If ElementType is null, then it means the base is a pointer
// (neither CAT nor VAT) and we'll attempt to get ElementType again
// for next iteration.
```
But, that can't ever happen because of the if stmt further up:
```
  if (!OASE)
continue;
```
So it might be worth doing some archaeology to figure out why/when this was 
broken.

JFYI!

https://github.com/llvm/llvm-project/pull/157443
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [openmp] [OpenMP] Taskgraph Clang 'record and replay' frontend support (PR #159774)

2025-09-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/159774

>From 861dfa4279cb0080dc714e64b7a2b50ac74485c4 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Tue, 16 Sep 2025 04:16:15 -0500
Subject: [PATCH 1/5] [OpenMP] Make loop index unsigned in
 __kmpc_omp_task_with_deps/__kmp_omp_task

NFC.

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp_taskdeps.cpp | 2 +-
 openmp/runtime/src/kmp_tasking.cpp  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp 
b/openmp/runtime/src/kmp_taskdeps.cpp
index abbca752f0587..743d8ed093c61 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -714,7 +714,7 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, 
kmp_int32 gtid,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index 37836fb457537..a3c7439593d5c 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1816,7 +1816,7 @@ kmp_int32 __kmp_omp_task(kmp_int32 gtid, kmp_task_t 
*new_task,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;

>From 94fe5ecc2f4b9a4a2a068b45358eff20feacfec6 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Mon, 15 Sep 2025 05:13:20 -0500
Subject: [PATCH 2/5] [OpenMP] Use ID not index to identify taskgraphs in
 libomp runtime

In preparation for the following patches, this patch changes the key
used to identify taskgraphs from a monotonic index into an ID (stored
in a linear table).

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp.h   |  5 ++-
 openmp/runtime/src/kmp_global.cpp  |  2 +-
 openmp/runtime/src/kmp_tasking.cpp | 72 --
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index 83afc0e83f231..4c4e9b44c1b2a 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -2678,7 +2678,7 @@ typedef struct kmp_tdg_info {
 extern int __kmp_tdg_dot;
 extern kmp_int32 __kmp_max_tdgs;
 extern kmp_tdg_info_t **__kmp_global_tdgs;
-extern kmp_int32 __kmp_curr_tdg_idx;
+extern kmp_int32 __kmp_curr_tdg_id;
 extern kmp_int32 __kmp_successors_size;
 extern std::atomic __kmp_tdg_task_id;
 extern kmp_int32 __kmp_num_tdg;
@@ -4392,6 +4392,9 @@ KMP_EXPORT kmp_int32 __kmpc_start_record_task(ident_t 
*loc, kmp_int32 gtid,
   kmp_int32 tdg_id);
 KMP_EXPORT void __kmpc_end_record_task(ident_t *loc, kmp_int32 gtid,
kmp_int32 input_flags, kmp_int32 
tdg_id);
+KMP_EXPORT void __kmpc_taskgraph(ident_t *loc_ref, kmp_int32 gtid,
+ kmp_int32 input_flags, kmp_uint32 tdg_id,
+ void (*entry)(void *), void *args);
 #endif
 /* Interface to fast scalable reduce methods routines */
 
diff --git a/openmp/runtime/src/kmp_global.cpp 
b/openmp/runtime/src/kmp_global.cpp
index 323d13e948b42..fdf7569116578 100644
--- a/openmp/runtime/src/kmp_global.cpp
+++ b/openmp/runtime/src/kmp_global.cpp
@@ -556,7 +556,7 @@ int *__kmp_nesting_nth_level;
 int __kmp_tdg_dot = 0;
 kmp_int32 __kmp_max_tdgs = 100;
 kmp_tdg_info_t **__kmp_global_tdgs = NULL;
-kmp_int32 __kmp_curr_tdg_idx =
+kmp_int32 __kmp_curr_tdg_id =
 0; // Id of the current TDG being recorded or executed
 kmp_int32 __kmp_num_tdg = 0;
 kmp_int32 __kmp_successors_size = 10; // Initial succesor size list for
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index a3c7439593d5c..a623f9f0be513 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1431,11 +1431,11 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, 
kmp_int32 gtid,
   }
 
 #if OMPX_TASKGRAPH
-  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_curr_tdg_idx);
+  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_curr_tdg_id);
   if (tdg && __kmp_tdg_is_recording(tdg->tdg_status) &&
   (task_entry != (kmp_routine_entry_t)__kmp_taskloop_task)) {
 taskdata->is_taskgraph = 1;
-taskdata->tdg = __kmp_global_tdgs[__kmp_curr_tdg_idx];
+taskdata->tdg = tdg;
 taskdata->td_task_id = KMP_GEN_TASK_ID();
 taskdata->td_tdg_task_id = KMP_ATOMIC_INC(&__kmp_tdg_task_id);
   }
@@ -2365,9 +2365,9 @@ without help of the runtime library.
 */
 void *__kmpc_task_reduction_init(int gtid, int num, void *data

[clang] [llvm] [openmp] [OpenMP] Taskgraph Clang 'record and replay' frontend support (PR #159774)

2025-09-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 created 
https://github.com/llvm/llvm-project/pull/159774

This is a version of the `ompx taskgraph` support posted in 
[PR66919](https://github.com/llvm/llvm-project/pull/66919), adapted to the 
official OpenMP 6.0 spelling of `omp taskgraph`, and with the `ompx` extension 
parts removed.

I've merged the changes to the current mainline, tidied up the patch series and 
adjusted tests a little, but other than that I've not done much testing, so 
support should still be considered experimental. See also 
https://github.com/llvm/llvm-project/issues/130926.

>From 861dfa4279cb0080dc714e64b7a2b50ac74485c4 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Tue, 16 Sep 2025 04:16:15 -0500
Subject: [PATCH 1/5] [OpenMP] Make loop index unsigned in
 __kmpc_omp_task_with_deps/__kmp_omp_task

NFC.

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp_taskdeps.cpp | 2 +-
 openmp/runtime/src/kmp_tasking.cpp  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp 
b/openmp/runtime/src/kmp_taskdeps.cpp
index abbca752f0587..743d8ed093c61 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -714,7 +714,7 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, 
kmp_int32 gtid,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index 37836fb457537..a3c7439593d5c 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1816,7 +1816,7 @@ kmp_int32 __kmp_omp_task(kmp_int32 gtid, kmp_task_t 
*new_task,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;

>From 858b6af35420f264b04918e738c1de2a63bd4d71 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Mon, 15 Sep 2025 05:13:20 -0500
Subject: [PATCH 2/5] [OpenMP] Use ID not index to identify taskgraphs in
 libomp runtime

In preparation for the following patches, this patch changes the key
used to identify taskgraphs from a monotonic index into an ID (stored
in a linear table).

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp.h   |  5 ++-
 openmp/runtime/src/kmp_global.cpp  |  2 +-
 openmp/runtime/src/kmp_tasking.cpp | 69 +-
 3 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index 83afc0e83f231..4c4e9b44c1b2a 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -2678,7 +2678,7 @@ typedef struct kmp_tdg_info {
 extern int __kmp_tdg_dot;
 extern kmp_int32 __kmp_max_tdgs;
 extern kmp_tdg_info_t **__kmp_global_tdgs;
-extern kmp_int32 __kmp_curr_tdg_idx;
+extern kmp_int32 __kmp_curr_tdg_id;
 extern kmp_int32 __kmp_successors_size;
 extern std::atomic __kmp_tdg_task_id;
 extern kmp_int32 __kmp_num_tdg;
@@ -4392,6 +4392,9 @@ KMP_EXPORT kmp_int32 __kmpc_start_record_task(ident_t 
*loc, kmp_int32 gtid,
   kmp_int32 tdg_id);
 KMP_EXPORT void __kmpc_end_record_task(ident_t *loc, kmp_int32 gtid,
kmp_int32 input_flags, kmp_int32 
tdg_id);
+KMP_EXPORT void __kmpc_taskgraph(ident_t *loc_ref, kmp_int32 gtid,
+ kmp_int32 input_flags, kmp_uint32 tdg_id,
+ void (*entry)(void *), void *args);
 #endif
 /* Interface to fast scalable reduce methods routines */
 
diff --git a/openmp/runtime/src/kmp_global.cpp 
b/openmp/runtime/src/kmp_global.cpp
index 323d13e948b42..fdf7569116578 100644
--- a/openmp/runtime/src/kmp_global.cpp
+++ b/openmp/runtime/src/kmp_global.cpp
@@ -556,7 +556,7 @@ int *__kmp_nesting_nth_level;
 int __kmp_tdg_dot = 0;
 kmp_int32 __kmp_max_tdgs = 100;
 kmp_tdg_info_t **__kmp_global_tdgs = NULL;
-kmp_int32 __kmp_curr_tdg_idx =
+kmp_int32 __kmp_curr_tdg_id =
 0; // Id of the current TDG being recorded or executed
 kmp_int32 __kmp_num_tdg = 0;
 kmp_int32 __kmp_successors_size = 10; // Initial succesor size list for
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index a3c7439593d5c..fc47daf469df3 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1431,11 +1431,11 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, 
kmp_int32 gtid,
   }
 
 #if OMPX_TASKGRAPH
-  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_curr_tdg_idx);
+  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_

[clang] [llvm] [openmp] [OpenMP] Taskgraph Clang 'record and replay' frontend support (PR #159774)

2025-09-19 Thread Julian Brown via cfe-commits


https://github.com/jtb20 updated 
https://github.com/llvm/llvm-project/pull/159774

>From 861dfa4279cb0080dc714e64b7a2b50ac74485c4 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Tue, 16 Sep 2025 04:16:15 -0500
Subject: [PATCH 1/5] [OpenMP] Make loop index unsigned in
 __kmpc_omp_task_with_deps/__kmp_omp_task

NFC.

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp_taskdeps.cpp | 2 +-
 openmp/runtime/src/kmp_tasking.cpp  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/openmp/runtime/src/kmp_taskdeps.cpp 
b/openmp/runtime/src/kmp_taskdeps.cpp
index abbca752f0587..743d8ed093c61 100644
--- a/openmp/runtime/src/kmp_taskdeps.cpp
+++ b/openmp/runtime/src/kmp_taskdeps.cpp
@@ -714,7 +714,7 @@ kmp_int32 __kmpc_omp_task_with_deps(ident_t *loc_ref, 
kmp_int32 gtid,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index 37836fb457537..a3c7439593d5c 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1816,7 +1816,7 @@ kmp_int32 __kmp_omp_task(kmp_int32 gtid, kmp_task_t 
*new_task,
 
 __kmp_free(old_record);
 
-for (kmp_int i = old_size; i < new_size; i++) {
+for (kmp_uint i = old_size; i < new_size; i++) {
   kmp_int32 *successorsList = (kmp_int32 *)__kmp_allocate(
   __kmp_successors_size * sizeof(kmp_int32));
   new_record[i].task = nullptr;

>From 94fe5ecc2f4b9a4a2a068b45358eff20feacfec6 Mon Sep 17 00:00:00 2001
From: Julian Brown 
Date: Mon, 15 Sep 2025 05:13:20 -0500
Subject: [PATCH 2/5] [OpenMP] Use ID not index to identify taskgraphs in
 libomp runtime

In preparation for the following patches, this patch changes the key
used to identify taskgraphs from a monotonic index into an ID (stored
in a linear table).

Co-authored-by: Adrian Munera 
---
 openmp/runtime/src/kmp.h   |  5 ++-
 openmp/runtime/src/kmp_global.cpp  |  2 +-
 openmp/runtime/src/kmp_tasking.cpp | 72 --
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index 83afc0e83f231..4c4e9b44c1b2a 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -2678,7 +2678,7 @@ typedef struct kmp_tdg_info {
 extern int __kmp_tdg_dot;
 extern kmp_int32 __kmp_max_tdgs;
 extern kmp_tdg_info_t **__kmp_global_tdgs;
-extern kmp_int32 __kmp_curr_tdg_idx;
+extern kmp_int32 __kmp_curr_tdg_id;
 extern kmp_int32 __kmp_successors_size;
 extern std::atomic __kmp_tdg_task_id;
 extern kmp_int32 __kmp_num_tdg;
@@ -4392,6 +4392,9 @@ KMP_EXPORT kmp_int32 __kmpc_start_record_task(ident_t 
*loc, kmp_int32 gtid,
   kmp_int32 tdg_id);
 KMP_EXPORT void __kmpc_end_record_task(ident_t *loc, kmp_int32 gtid,
kmp_int32 input_flags, kmp_int32 
tdg_id);
+KMP_EXPORT void __kmpc_taskgraph(ident_t *loc_ref, kmp_int32 gtid,
+ kmp_int32 input_flags, kmp_uint32 tdg_id,
+ void (*entry)(void *), void *args);
 #endif
 /* Interface to fast scalable reduce methods routines */
 
diff --git a/openmp/runtime/src/kmp_global.cpp 
b/openmp/runtime/src/kmp_global.cpp
index 323d13e948b42..fdf7569116578 100644
--- a/openmp/runtime/src/kmp_global.cpp
+++ b/openmp/runtime/src/kmp_global.cpp
@@ -556,7 +556,7 @@ int *__kmp_nesting_nth_level;
 int __kmp_tdg_dot = 0;
 kmp_int32 __kmp_max_tdgs = 100;
 kmp_tdg_info_t **__kmp_global_tdgs = NULL;
-kmp_int32 __kmp_curr_tdg_idx =
+kmp_int32 __kmp_curr_tdg_id =
 0; // Id of the current TDG being recorded or executed
 kmp_int32 __kmp_num_tdg = 0;
 kmp_int32 __kmp_successors_size = 10; // Initial succesor size list for
diff --git a/openmp/runtime/src/kmp_tasking.cpp 
b/openmp/runtime/src/kmp_tasking.cpp
index a3c7439593d5c..a623f9f0be513 100644
--- a/openmp/runtime/src/kmp_tasking.cpp
+++ b/openmp/runtime/src/kmp_tasking.cpp
@@ -1431,11 +1431,11 @@ kmp_task_t *__kmp_task_alloc(ident_t *loc_ref, 
kmp_int32 gtid,
   }
 
 #if OMPX_TASKGRAPH
-  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_curr_tdg_idx);
+  kmp_tdg_info_t *tdg = __kmp_find_tdg(__kmp_curr_tdg_id);
   if (tdg && __kmp_tdg_is_recording(tdg->tdg_status) &&
   (task_entry != (kmp_routine_entry_t)__kmp_taskloop_task)) {
 taskdata->is_taskgraph = 1;
-taskdata->tdg = __kmp_global_tdgs[__kmp_curr_tdg_idx];
+taskdata->tdg = tdg;
 taskdata->td_task_id = KMP_GEN_TASK_ID();
 taskdata->td_tdg_task_id = KMP_ATOMIC_INC(&__kmp_tdg_task_id);
   }
@@ -2365,9 +2365,9 @@ without help of the runtime library.
 */
 void *__kmpc_task_reduction_init(int gtid, int num, void *data

[clang] [llvm] [openmp] [OpenMP] Taskgraph Clang 'record and replay' frontend support (PR #159774)

2025-09-19 Thread Julian Brown via cfe-commits


jtb20 wrote:

The location hash used in the `print_omp` tests is very sensitive to code 
changes (and may not even generate unique IDs sufficiently well), so that 
probably wants attention (at least as a follow-up patch at some point).

https://github.com/llvm/llvm-project/pull/159774
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

74 matches

Mail list logo