[cfe-commits] [PATCH] First OpenMP patch

[Benjamin Kramer]

On 21.11.2012, at 15:36, Mahesha HS <mahesha.llvm at gmail.com> wrote:

> Hi all,
> 
> Here is the first OpenMP patch.
> 
> It supports options -fopenmp and -fno-openmp. When only -fopenmp is
> passed, OpenMP is enabled. When only -fno-openmp is passed, OpenMP is
> disabled. When both the options are passed, the one which appear later
> in the command line wins.
> 
> I am re-sending this patch third time. I am positively hoping that -
> this time - this patch will get checked-in into trunk :)


> Index: include/clang/Driver/Options.td
> ===================================================================
> --- include/clang/Driver/Options.td	(revision 168426)
> +++ include/clang/Driver/Options.td	(working copy)
> @@ -590,7 +590,8 @@
>  def fobjc_sender_dependent_dispatch : Flag<["-"], "fobjc-sender-dependent-dispatch">, Group<f_Group>;
>  def fobjc : Flag<["-"], "fobjc">, Group<f_Group>;
>  def fomit_frame_pointer : Flag<["-"], "fomit-frame-pointer">, Group<f_Group>;
> -def fopenmp : Flag<["-"], "fopenmp">, Group<f_Group>;
> +def fopenmp : Flag<["-"], "fopenmp">, Group<f_Group>, Flags<[CC1Option]>;
> +def fno_openmp : Flag<["-"], "fno-openmp">, Group<f_Group>, Flags<[CC1Option]>;
>  def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, Group<f_Group>;
>  def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, Group<f_Group>;
>  def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;
> Index: include/clang/Basic/LangOptions.def
> ===================================================================
> --- include/clang/Basic/LangOptions.def	(revision 168426)
> +++ include/clang/Basic/LangOptions.def	(working copy)
> @@ -168,6 +168,10 @@
>  
>  BENIGN_LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, "retain documentation comments from system headers in the AST")
>  
> +/// OpenMP
> +LANGOPT(OpenMP, 1, 0, "Enables OpenMP support.")
> +LANGOPT(NoOpenMP, 1, 0, "Disables OpenMP support.")
> +

Why do you need two lang options for a binary flag? They are mutually exclusive, no need for the negative form.

>  /// Runtime sanitizers.
>  #define SANITIZER(NAME, ID) \
>  BENIGN_LANGOPT(Sanitize##ID, 1, 0, NAME " sanitizer")
> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp	(revision 168426)
> +++ lib/Driver/Tools.cpp	(working copy)
> @@ -2504,6 +2504,16 @@
>  
>    Args.AddLastArg(CmdArgs, options::OPT_pthread);
>  
> +  // When both -fopenmp and -fno-openmp options are present,
> +  // later one wins.
> +  if (Arg *A = Args.getLastArg(options::OPT_fopenmp,
> +                               options::OPT_fno_openmp)) {
> +    if (A->getOption().matches(options::OPT_fopenmp)) {
> +      CmdArgs.push_back("-fopenmp");
> +    } else {
> +      CmdArgs.push_back("-fno-openmp");
> +    }
> +  }

One of those two should be the default and the corresponding branch could be dropped.

>  
>    // -stack-protector=0 is default.
>    unsigned StackProtectorLevel = 0;
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp	(revision 168426)
> +++ lib/Frontend/CompilerInvocation.cpp	(working copy)
> @@ -1239,6 +1239,11 @@
>    Opts.ApplePragmaPack = Args.hasArg(OPT_fapple_pragma_pack);
>    Opts.CurrentModule = Args.getLastArgValue(OPT_fmodule_name);
>  
> +  // If -fopenmp enablement fails, then check, if -fno-openmp is need to be 
> +  // enabled.
> +  if (!(Opts.OpenMP = Args.hasFlag(OPT_fopenmp, OPT_fno_openmp, false)))
> +    Opts.NoOpenMP = Args.hasFlag(OPT_fno_openmp, OPT_fopenmp, false);
> +

Could be simplified if there is only one LangOption.

- Ben

>    // Record whether the __DEPRECATED define was requested.
>    Opts.Deprecated = Args.hasFlag(OPT_fdeprecated_macro,
>                                   OPT_fno_deprecated_macro,
> Index: test/Driver/openmp-options.c
> ===================================================================
> --- test/Driver/openmp-options.c	(revision 0)
> +++ test/Driver/openmp-options.c	(revision 0)
> @@ -0,0 +1,14 @@
> +// Neither -fopenmp nor -fno-openmp is passed.
> +// RUN: %clang -### -S -o %t %s  2>&1 | not grep -w -- -fopenmp && not grep -w -- -fno-openmp
> +
> +// Only -fopenmp is passed.
> +// RUN: %clang -### -S -o %t %s -fopenmp  2>&1 | grep -w -- -fopenmp && not grep -w -- -fno-openmp
> +
> +// Both -fopenmp and -fno-openmp are passed. But, -fopenmp wins as it appears after -fno-openmp.
> +// RUN: %clang -### -S -o %t %s -fno-openmp -fopenmp  2>&1 | grep -w -- -fopenmp && not grep -w -- -fno-openmp
> +
> +// Only -fno-openmp is passed.
> +// RUN: %clang -### -S -o %t %s -fno-openmp  2>&1 | grep -w -- -fno-openmp && not grep -w -- -fopenmp
> +
> +// Both -fopenmp and -fno-openmp are passed. But, -fno-openmp wins as it appears after -fopenmp.
> +// RUN: %clang -### -S -o %t %s -fopenmp -fno-openmp  2>&1 | grep -w -- -fno-openmp && not grep -w -- -fopenmp