[LLVMdev] [cfe-dev] OpenMP support in CLANG: A proposal

[Eli Friedman]

On Sat, Oct 13, 2012 at 9:06 AM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
> Hi Eli,
>
> Attached zipped file, named, "fopenmp_option_support.tar.gz" contains
> the first patch, along with relevant *test case*. This patch is to
> support the option "-fopenmp" option in Clang.
>
> Following files are changed in this patch. Please start going through
> this patch, and let me know comments. Meanwhile, I will prepare next
> patch.

In the future, please include tests inside the .patch file (you can
use svn add to tell svn to include it in diffs).

// clang should *not* emit the below warning:
// warning: argument unused during compilation: '-fopenmp'
//
// RUN: %clang -c %s 2> %t.log
//
// RUN: grep "warning: argument unused during compilation: '-fopenmp'"
%t.log | count 0

This test is way too specific to be useful.

A test that checks the appropriate option is passed to clang -cc1
would be appropriate.

+def omp_pragma_ignored : Warning<
+  "omp pragma ignored; did you forget to add '-fopenmp' flag?">;

This warning appears to be unused.  (Please try to add warnings in the
same patch they are used.)

+LANGOPT(OpenMpOn, 1, 0, "Enables OpenMP support.")

Please just name this OpenMP; the "On" doesn't add anything.

+  // Support for OpenMP: Implementation of 'fopenmp' option passing
+  // mechanism.
+  if (Opts.OpenMpOn)
+    Res.push_back("-fopenmp");

This is overly verbose; a very short comment like "OpenMP support", or
even no comment at all, would be appropriate.

Please send future patches to cfe-commits; these patches aren't of
general interest to all of cfe-dev and llvmdev.

-Eli