On 29.10.21 13:06, Jakub Jelinek wrote:
On Thu, Oct 28, 2021 at 05:51:59PM +0200, Tobias Burnus wrote:
libcpp/ChangeLog:

     PR c++/102409
     * directives.c (destringize_and_run): Add PRAGMA_OP to the
     CPP_PRAGMA token's flags to mark is as coming from _Pragma.
     * include/cpplib.h (PRAGMA_OP): #define, to be used with token flags.
     * macro.c (collect_args): Only handle CPP_PRAGMA special if PRAGMA_OP
     is set.

The patch itself looks reasonable to me, but it should come up with
testsuite coverage.  And the testsuite coverage should include both normal
testcases that do use integrated preprocessor, and the same with
-save-temps to make sure that even when preprocessing separately it works
too.

Yes, I realized myself that I missed to include a testcase – thanks for
the -save-temps suggestion!

Updated patch enclosed.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955
libcpp: Fix _Pragma expansion [PR102409]

Both #pragma and _Pragma ended up as CPP_PRAGMA. Presumably since
r131819 (2008, GCC 4.3) for PR34692, pragmas are not expanded in
macro arguments but are output as is before. From the old bug report,
that was to fix usage like
  FOO (
    #pragma GCC diagnostic
  )
However, that change also affected _Pragma such that
  BAR (
    "1";
    _Pragma("omp ..."); )
yielded
  #pragma omp ...
followed by what BAR expanded too, possibly including '"1";'.

This commit adds a flag, PRAGMA_OP, to tokens to make the two
distinguishable - and include again _Pragma in the expanded arguments.

libcpp/ChangeLog:

	PR c++/102409
	* directives.c (destringize_and_run): Add PRAGMA_OP to the
	CPP_PRAGMA token's flags to mark is as coming from _Pragma.
	* include/cpplib.h (PRAGMA_OP): #define, to be used with token flags.
	* macro.c (collect_args): Only handle CPP_PRAGMA special if PRAGMA_OP
	is set.

gcc/testsuite/ChangeLog:

	* c-c++-common/gomp/pragma-1.c: New test.
	* c-c++-common/gomp/pragma-2.c: New test.

 gcc/testsuite/c-c++-common/gomp/pragma-1.c | 50 ++++++++++++++++++++++++++++++
 gcc/testsuite/c-c++-common/gomp/pragma-2.c | 50 ++++++++++++++++++++++++++++++
 libcpp/directives.c                        |  2 ++
 libcpp/include/cpplib.h                    |  1 +
 libcpp/macro.c                             |  2 +-
 5 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/c-c++-common/gomp/pragma-1.c b/gcc/testsuite/c-c++-common/gomp/pragma-1.c
new file mode 100644
index 00000000000..e330f17204a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/pragma-1.c
@@ -0,0 +1,50 @@
+/* { dg-additional-options "-fdump-tree-original" }  */
+/* PR c++/51484  */
+
+#define TEST(T) { \
+  int fail = 0, trial; \
+  for (int trial = 0; trial < TRIALS && fail == 0; trial++) { \
+    _Pragma("omp target teams num_teams(1) thread_limit(1024)") \
+     {T} \
+  } \
+}
+
+#define TRIALS (1)
+#define N (1024*3)
+
+int main(void) {
+
+  double C[N], D[N];
+  double S[N];
+  double p[2];
+  int i; 
+  for (i = 0; i < N; i++)
+  {C[i] = 1; D[i] = i;}
+
+  int max_threads = 224;
+
+#define PARALLEL(X) TEST({ \
+_Pragma("omp parallel if(threads[0] > 1) num_threads(threads[0])") \
+{ \
+_Pragma("omp for ordered") \
+  X  \
+_Pragma("omp for schedule(auto) ordered") \
+  X  \
+} \
+})
+
+  for (int t = 0; t <= max_threads; t += max_threads) {
+    int threads[1]; threads[0] = t;
+    S[0] = 0;
+    PARALLEL(
+    for (int i = 0; i < N; i++) { \
+      _Pragma("omp ordered") \
+      S[0] += C[i] + D[i]; \
+    })
+  }
+  return 0;
+}
+
+/* On expansion, the _Pragma were wrongly placed, ensure the order is now correct: */
+/* { dg-final { scan-tree-dump "#pragma omp target.*#pragma omp teams num_teams\\(1\\) thread_limit\\(1024\\).*#pragma omp parallel num_threads\\(threads\\\[0\\\]\\) if\\(threads\\\[0\\\] > 1\\).*#pragma omp for ordered.*#pragma omp ordered.*#pragma omp for ordered schedule\\(auto\\).*#pragma omp ordered" "original" } } */
+
diff --git a/gcc/testsuite/c-c++-common/gomp/pragma-2.c b/gcc/testsuite/c-c++-common/gomp/pragma-2.c
new file mode 100644
index 00000000000..5358f875959
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/pragma-2.c
@@ -0,0 +1,50 @@
+/* { dg-additional-options "-fdump-tree-original -save-temps" }  */
+/* PR c++/51484  */
+
+#define TEST(T) { \
+  int fail = 0, trial; \
+  for (int trial = 0; trial < TRIALS && fail == 0; trial++) { \
+    _Pragma("omp target teams num_teams(1) thread_limit(1024)") \
+     {T} \
+  } \
+}
+
+#define TRIALS (1)
+#define N (1024*3)
+
+int main(void) {
+
+  double C[N], D[N];
+  double S[N];
+  double p[2];
+  int i; 
+  for (i = 0; i < N; i++)
+  {C[i] = 1; D[i] = i;}
+
+  int max_threads = 224;
+
+#define PARALLEL(X) TEST({ \
+_Pragma("omp parallel if(threads[0] > 1) num_threads(threads[0])") \
+{ \
+_Pragma("omp for ordered") \
+  X  \
+_Pragma("omp for schedule(auto) ordered") \
+  X  \
+} \
+})
+
+  for (int t = 0; t <= max_threads; t += max_threads) {
+    int threads[1]; threads[0] = t;
+    S[0] = 0;
+    PARALLEL(
+    for (int i = 0; i < N; i++) { \
+      _Pragma("omp ordered") \
+      S[0] += C[i] + D[i]; \
+    })
+  }
+  return 0;
+}
+
+/* On expansion, the _Pragma were wrongly placed, ensure the order is now correct: */
+/* { dg-final { scan-tree-dump "#pragma omp target.*#pragma omp teams num_teams\\(1\\) thread_limit\\(1024\\).*#pragma omp parallel num_threads\\(threads\\\[0\\\]\\) if\\(threads\\\[0\\\] > 1\\).*#pragma omp for ordered.*#pragma omp ordered.*#pragma omp for ordered schedule\\(auto\\).*#pragma omp ordered" "original" } } */
+
diff --git a/libcpp/directives.c b/libcpp/directives.c
index b4bc8b4df30..34f7677f718 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1907,6 +1907,8 @@ destringize_and_run (cpp_reader *pfile, const cpp_string *in,
   save_directive = pfile->directive;
   pfile->directive = &dtable[T_PRAGMA];
   do_pragma (pfile);
+  if (pfile->directive_result.type == CPP_PRAGMA)
+    pfile->directive_result.flags |= PRAGMA_OP;
   end_directive (pfile, 1);
   pfile->directive = save_directive;
 
diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h
index 6e2fcb6b1f2..56b07acc1d7 100644
--- a/libcpp/include/cpplib.h
+++ b/libcpp/include/cpplib.h
@@ -198,6 +198,7 @@ struct GTY(()) cpp_string {
 				    operator, or before this token
 				    after a # operator.  */
 #define NO_EXPAND	(1 << 10) /* Do not macro-expand this token.  */
+#define PRAGMA_OP	(1 << 11) /* _Pragma token.  */
 
 /* Specify which field, if any, of the cpp_token union is used.  */
 
diff --git a/libcpp/macro.c b/libcpp/macro.c
index f214548de1e..b2f797cae35 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -1259,7 +1259,7 @@ collect_args (cpp_reader *pfile, const cpp_hashnode *node,
 	  else if (token->type == CPP_EOF
 		   || (token->type == CPP_HASH && token->flags & BOL))
 	    break;
-	  else if (token->type == CPP_PRAGMA)
+	  else if (token->type == CPP_PRAGMA && !(token->flags & PRAGMA_OP))
 	    {
 	      cpp_token *newtok = _cpp_temp_token (pfile);
 

Reply via email to