[cfe-commits] [PATCH] First OpenMP patch

[Mahesha HS]

Hi Ben,

Though -fopenmp and -fno-openmp are mutually exclusive options,
underlying functionality, that we intended to implement differ a
little. In a nutshell, we, sometime back discussed in CFE DEV list to
implement OpenMP as follows. We thought that passing neither -fopenmp
nor -fno-openmp  is not same as passing only -fno-openmp.  Following
is the intended implementation. This forced me to add two lang options
as I could not handle it using only one lang option. If this was not
there, I guess, all your comments would not have araised.

if (source contains OpenMP statements) {
     if (neither -fopenmp nor -fno-openmp present) {
         Throw warning.
     }
     else if (only -fopenmp present) {
         Process OpenMP
     }
     else if (only -fno-openmp) {
        Silently ignore OpenMP. Do not throw warning.
     }
    else if (both -fopenmp and -fno-openmp are present) {
        Decide based on which one appears later in the command line.
    }
}

--
mahesha


On Wed, Nov 21, 2012 at 8:28 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
> 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
>
>
>
>



-- 
mahesha