<html><body>
<p><font size="2" face="sans-serif">Hi, thanks for reviewing the patch!</font><br>
<br>
<tt><font size="2">metafoo@gmail.com wrote on 06/15/2015 02:44:12 PM:<br>
<br>
> From: Richard Smith <richard@metafoo.co.uk></font></tt><br>
<tt><font size="2">> To: reviews+D10446+public+80453832d50bde4c@reviews.llvm.org</font></tt><br>
<tt><font size="2">> Cc: Samuel F Antao/Watson/IBM@IBMUS, Alexey Bataev <br>
> <a.bataev@hotmail.com>, Hal Finkel <hfinkel@anl.gov>, cfe commits <br>
> <cfe-commits@cs.uiuc.edu></font></tt><br>
<tt><font size="2">> Date: 06/15/2015 02:44 PM</font></tt><br>
<tt><font size="2">> Subject: Re: [PATCH] Expand macros in OpenMP pragmas.</font></tt><br>
<tt><font size="2">> Sent by: metafoo@gmail.com</font></tt><br>
<tt><font size="2">> <br>
> On Mon, Jun 15, 2015 at 9:30 AM, Samuel Antao <sfantao@us.ibm.com> wrote:</font></tt><br>
<tt><font size="2">> Hi ABataev, hfinkel, rsmith,<br>
> <br>
> According to the OpenMP spec, all the preprocessor macros should be <br>
> expanded in OpenMP pragmas. This patch adds support for that.</font></tt><br>
<tt><font size="2">> <br>
> We already have support for that :) I assume you mean this patch <br>
> adds support for that *to -E*.</font></tt><br>
<br>
<tt><font size="2">You're right, I guess that would be a more accurate description of what the patch is trying to address. :)</font></tt><br>
<br>
<tt><font size="2">> </font></tt><br>
<tt><font size="2">> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10446&d=AwMFAw&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=BeAAgIVvme4pAQSse5Pdjv5ndI9OhcYgfRSq20YTE7k&s=Woa1QoKf_woTQUhg7rER2mN20NEZfVuLvbrVmqKCOto&e=">http://reviews.llvm.org/D10446</a><br>
> <br>
> Files:<br>
> lib/Frontend/PrintPreprocessedOutput.cpp<br>
> test/OpenMP/preprocessor.c<br>
> <br>
> Index: lib/Frontend/PrintPreprocessedOutput.cpp<br>
> ===================================================================<br>
> --- lib/Frontend/PrintPreprocessedOutput.cpp<br>
> +++ lib/Frontend/PrintPreprocessedOutput.cpp<br>
> @@ -588,6 +588,37 @@<br>
> Callbacks->setEmittedDirectiveOnThisLine();<br>
> }<br>
> };<br>
> +struct OMPPragmaHandler : public PragmaHandler {<br>
> + const char *Prefix;<br>
> + PrintPPOutputPPCallbacks *Callbacks;<br>
> +<br>
> + OMPPragmaHandler(const char *prefix, PrintPPOutputPPCallbacks *callbacks)<br>
> + : Prefix(prefix), Callbacks(callbacks) {}<br>
> + void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,<br>
> + Token &PragmaTok) override {<br>
> +<br>
> + // The tokens after pragma omp need to be expanded.<br>
> + //<br>
> + // OpenMP [2.1, Directive format]<br>
> + // Preprocessing tokens following the #pragma omp are subject to macro<br>
> + // replacement.<br>
> +<br>
> + // Figure out what line we went to and insert the appropriate number of<br>
> + // newline characters.<br>
> + Callbacks->startNewLineIfNeeded();<br>
> + Callbacks->MoveToLine(PragmaTok.getLocation());<br>
> + Callbacks->OS.write(Prefix, strlen(Prefix));<br>
> + // Read and print all of the pragma tokens.<br>
> + while (PragmaTok.isNot(tok::eod)) {<br>
> + if (PragmaTok.hasLeadingSpace())<br>
> + Callbacks->OS << ' ';</font></tt><br>
<tt><font size="2">> <br>
> Since you're doing macro expansion here, there are other ways in <br>
> which you can need a leading space before a token; you should also <br>
> insert a space if Callbacks->AvoidConcat says you need one (this is <br>
> a pre-existing bug in the UnknownPragmaHandler).</font></tt><br>
<br>
<tt><font size="2">Ok, now checking if AvoidConcat has anything to say about required white spaces.</font></tt><br>
<br>
<tt><font size="2">> <br>
> + std::string TokSpell = PP.getSpelling(PragmaTok);<br>
> + Callbacks->OS.write(&TokSpell[0], TokSpell.size());<br>
> + PP.Lex(PragmaTok);<br>
> + }<br>
> + Callbacks->setEmittedDirectiveOnThisLine();<br>
> + }<br>
> +};<br>
> } // end anonymous namespace<br>
> <br>
> <br>
> @@ -722,6 +753,7 @@<br>
> PP.AddPragmaHandler("GCC", new UnknownPragmaHandler("#pragma <br>
> GCC",Callbacks));<br>
> PP.AddPragmaHandler("clang",<br>
> new UnknownPragmaHandler("#pragma clang", Callbacks));<br>
> + PP.AddPragmaHandler("omp", new OMPPragmaHandler("#pragma omp", Callbacks));</font></tt><br>
<tt><font size="2">> <br>
> Instead of adding a new kind of PragmaHandler here, let's generalize<br>
> the existing infrastructure: please add a bool flag to <br>
> UnknownPragmaHandler that says whether to expand macros, set it to <br>
> 'true' for omp and to getLangOpts().MicrosoftExt for the other <br>
> cases. (Using MicrosoftExt here isn't right, but it matches our <br>
> current behavior and we can correct it in a later change.)</font></tt><br>
<br>
<tt><font size="2">Ok, Done!</font></tt><br>
<br>
<tt><font size="2">> </font></tt><br>
<tt><font size="2">> PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));<br>
> <br>
> Index: test/OpenMP/preprocessor.c<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ test/OpenMP/preprocessor.c<br>
> @@ -0,0 +1,30 @@<br>
> +// RUN: %clang -fopenmp -E -o - %s 2>&1 | FileCheck %s</font></tt><br>
<tt><font size="2">> <br>
> This should be a %clang_cc1 test, and should go in test/Preprocessor.</font></tt><br>
<br>
<tt><font size="2">Done! Changed the test slightly to depend on white spaces mandated by AvoidConcat.</font></tt><br>
<br>
<tt><font size="2">> <br>
> +<br>
> +// This is to make sure the pragma name is not expanded!<br>
> +#define omp (0xDEADBEEF)<br>
> +<br>
> +#define N 2<br>
> +#define M 1<br>
> +<br>
> +#define map_to_be_expanded(x) map(tofrom:x)<br>
> +#define sched_to_be_expanded(x) schedule(x,N)<br>
> +#define reda_to_be_expanded(x) reduction(+:x)<br>
> +#define redb_to_be_expanded(x,op) reduction(op:x)<br>
> +<br>
> +void foo(int *a, int *b) {<br>
> + //CHECK: omp target map(a[0:2]) map(tofrom:b[0:2*1])<br>
> + #pragma omp target map(a[0:N]) map_to_be_expanded(b[0:2*M])<br>
> + {<br>
> + int reda;<br>
> + int redb;<br>
> + //CHECK: omp parallel for schedule(static,2) reduction(+:reda) <br>
> reduction(*:redb)<br>
> + #pragma omp parallel for sched_to_be_expanded(static) \<br>
> + reda_to_be_expanded(reda) redb_to_be_expanded(redb,*)<br>
> + for (int i = 0; i < N; ++i) {<br>
> + reda += a[i];<br>
> + redb += b[i];<br>
> + }<br>
> + a[0] = reda;<br>
> + b[0] = redb;<br>
> + }<br>
> +}<br>
> <br>
> EMAIL PREFERENCES<br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFAw&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=BeAAgIVvme4pAQSse5Pdjv5ndI9OhcYgfRSq20YTE7k&s=26dKBHLPTKtwU3CwkZDnwjXZQ_V0W9pZXXwsz3vgvpg&e=">http://reviews.llvm.org/settings/panel/emailpreferences/</a></font></tt></body></html>