[PATCH] Expand macros in OpenMP pragmas.

[Samuel F Antao]

Hi, thanks for reviewing the patch!

metafoo at gmail.com wrote on 06/15/2015 02:44:12 PM:

> From: Richard Smith <richard at metafoo.co.uk>
> To: reviews+D10446+public+80453832d50bde4c at reviews.llvm.org
> Cc: Samuel F Antao/Watson/IBM at IBMUS, Alexey Bataev
> <a.bataev at hotmail.com>, Hal Finkel <hfinkel at anl.gov>, cfe commits
> <cfe-commits at cs.uiuc.edu>
> Date: 06/15/2015 02:44 PM
> Subject: Re: [PATCH] Expand macros in OpenMP pragmas.
> Sent by: metafoo at gmail.com
>
> On Mon, Jun 15, 2015 at 9:30 AM, Samuel Antao <sfantao at us.ibm.com> wrote:
> Hi ABataev, hfinkel, rsmith,
>
> According to the OpenMP spec, all the preprocessor macros should be
> expanded in OpenMP pragmas. This patch adds support for that.
>
> We already have support for that :) I assume you mean this patch
> adds support for that *to -E*.

You're right, I guess that would be a more accurate description of what the
patch is trying to address. :)

>
> http://reviews.llvm.org/D10446
>
> Files:
>   lib/Frontend/PrintPreprocessedOutput.cpp
>   test/OpenMP/preprocessor.c
>
> Index: lib/Frontend/PrintPreprocessedOutput.cpp
> ===================================================================
> --- lib/Frontend/PrintPreprocessedOutput.cpp
> +++ lib/Frontend/PrintPreprocessedOutput.cpp
> @@ -588,6 +588,37 @@
>      Callbacks->setEmittedDirectiveOnThisLine();
>    }
>  };
> +struct OMPPragmaHandler : public PragmaHandler {
> +  const char *Prefix;
> +  PrintPPOutputPPCallbacks *Callbacks;
> +
> +  OMPPragmaHandler(const char *prefix, PrintPPOutputPPCallbacks
*callbacks)
> +      : Prefix(prefix), Callbacks(callbacks) {}
> +  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
> +                    Token &PragmaTok) override {
> +
> +    // The tokens after pragma omp need to be expanded.
> +    //
> +    //  OpenMP [2.1, Directive format]
> +    //  Preprocessing tokens following the #pragma omp are subject to
macro
> +    //  replacement.
> +
> +    // Figure out what line we went to and insert the appropriate number
of
> +    // newline characters.
> +    Callbacks->startNewLineIfNeeded();
> +    Callbacks->MoveToLine(PragmaTok.getLocation());
> +    Callbacks->OS.write(Prefix, strlen(Prefix));
> +    // Read and print all of the pragma tokens.
> +    while (PragmaTok.isNot(tok::eod)) {
> +      if (PragmaTok.hasLeadingSpace())
> +        Callbacks->OS << ' ';
>
> Since you're doing macro expansion here, there are other ways in
> which you can need a leading space before a token; you should also
> insert a space if Callbacks->AvoidConcat says you need one (this is
> a pre-existing bug in the UnknownPragmaHandler).

Ok, now checking if AvoidConcat has anything to say about required white
spaces.

>
> +      std::string TokSpell = PP.getSpelling(PragmaTok);
> +      Callbacks->OS.write(&TokSpell[0], TokSpell.size());
> +      PP.Lex(PragmaTok);
> +    }
> +    Callbacks->setEmittedDirectiveOnThisLine();
> +  }
> +};
>  } // end anonymous namespace
>
>
> @@ -722,6 +753,7 @@
>    PP.AddPragmaHandler("GCC", new UnknownPragmaHandler("#pragma
> GCC",Callbacks));
>    PP.AddPragmaHandler("clang",
>                        new UnknownPragmaHandler("#pragma clang",
Callbacks));
> +  PP.AddPragmaHandler("omp", new OMPPragmaHandler("#pragma omp",
Callbacks));
>
> Instead of adding a new kind of PragmaHandler here, let's generalize
> the existing infrastructure: please add a bool flag to
> UnknownPragmaHandler that says whether to expand macros, set it to
> 'true' for omp and to getLangOpts().MicrosoftExt for the other
> cases. (Using MicrosoftExt here isn't right, but it matches our
> current behavior and we can correct it in a later change.)

Ok, Done!

>
>    PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
>
> Index: test/OpenMP/preprocessor.c
> ===================================================================
> --- /dev/null
> +++ test/OpenMP/preprocessor.c
> @@ -0,0 +1,30 @@
> +// RUN:   %clang -fopenmp -E -o - %s 2>&1 | FileCheck %s
>
> This should be a %clang_cc1 test, and should go in test/Preprocessor.

Done! Changed the test slightly to depend on white spaces mandated by
AvoidConcat.

>
> +
> +// This is to make sure the pragma name is not expanded!
> +#define omp (0xDEADBEEF)
> +
> +#define N 2
> +#define M 1
> +
> +#define map_to_be_expanded(x) map(tofrom:x)
> +#define sched_to_be_expanded(x) schedule(x,N)
> +#define reda_to_be_expanded(x) reduction(+:x)
> +#define redb_to_be_expanded(x,op) reduction(op:x)
> +
> +void foo(int *a, int *b) {
> +  //CHECK: omp target map(a[0:2]) map(tofrom:b[0:2*1])
> +  #pragma omp target map(a[0:N]) map_to_be_expanded(b[0:2*M])
> +  {
> +    int reda;
> +    int redb;
> +    //CHECK: omp parallel for schedule(static,2) reduction(+:reda)
> reduction(*:redb)
> +    #pragma omp parallel for sched_to_be_expanded(static) \
> +        reda_to_be_expanded(reda) redb_to_be_expanded(redb,*)
> +    for (int i = 0; i < N; ++i) {
> +      reda += a[i];
> +      redb += b[i];
> +    }
> +    a[0] = reda;
> +    b[0] = redb;
> +  }
> +}
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150615/68ca712b/attachment.html>