[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added inline comments.


================
Comment at: lib/Driver/ToolChain.cpp:808
+          continue;
+      } else if (XOpenMPTargetNoTriple)
+        // Passing device args: -Xopenmp-target -opt=val.
----------------
hfinkel wrote:
> Please include {} around this else-if  code, even though it is not necessary, because the other blocks require it.
Done


================
Comment at: lib/Driver/ToolChain.cpp:820
+      if (!XOpenMPTargetArg || Index > Prev + 1) {
+        getDriver().Diag(diag::err_drv_invalid_Xopenmp_target_with_args)
+            << A->getAsString(Args);
----------------
hfinkel wrote:
> Is this covered by a test case?
Done


================
Comment at: lib/Driver/ToolChain.cpp:827
+              options::OPT_fopenmp_targets_EQ).size() != 1) {
+        getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
+        continue;
----------------
hfinkel wrote:
> Is this covered by a test case?
Done


================
Comment at: test/Driver/openmp-offload.c:615
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
----------------
hfinkel wrote:
> Now that this is in common code, why are these arguments still unused?
Fixed.


https://reviews.llvm.org/D34784