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

61 matches

Mail list logo