[PATCH] D9888: [OPENMP] Driver support for OpenMP offloading

Samuel Antao via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 18 10:32:35 PDT 2016


sfantao added a comment.

Hi Richard,

Thanks for your review. I partitioned some of the stuff I am proposing here in smaller patches:

http://reviews.llvm.org/D18170
http://reviews.llvm.org/D18171
http://reviews.llvm.org/D18172

These patches already try to incorporate the feedback I got in http://lists.llvm.org/pipermail/cfe-dev/2016-February/047547.html related with the generation of actions.

Thanks again,
Samuel


================
Comment at: include/clang/Driver/Options.td:1617-1618
@@ -1616,2 +1616,4 @@
   HelpText<"Write output to <file>">, MetaVarName<"<file>">;
+def omptargets_EQ : CommaJoined<["-"], "omptargets=">, Flags<[DriverOption, CC1Option]>,
+  HelpText<"Specify comma-separated list of triples OpenMP offloading targets to be supported">;
 def pagezero__size : JoinedOrSeparate<["-"], "pagezero_size">;
----------------
rsmith wrote:
> This is an unfortunate flag name; `-oblah` already means something. Is this name chosen for compatibility with some other system, or could we change it to, say, `-fopenmp-targets=`?
You are right, we are now using -fomptargets in codegen exactly because of that. I can change it to `-fopenmp-targets=` we don't have any compatibility issues at this point.

================
Comment at: lib/Driver/Tools.cpp:316
@@ +315,3 @@
+  // Add commands to embed target binaries. We ensure that each section and
+  // image s 16-byte aligned. This is not mandatory, but increases the
+  // likelihood of data to be aligned with a cache block in several main host
----------------
rsmith wrote:
> s -> is
I'll fix it.


http://reviews.llvm.org/D9888





More information about the cfe-commits mailing list