[PATCH] D34784: [OpenMP] Add flag for specifying the target device architecture for OpenMP device offloading
Hal Finkel via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 5 18:01:51 PDT 2017
hfinkel added inline comments.
================
Comment at: lib/Driver/ToolChain.cpp:808
+ continue;
+ } else if (XOpenMPTargetNoTriple)
+ // Passing device args: -Xopenmp-target -opt=val.
----------------
Please include {} around this else-if code, even though it is not necessary, because the other blocks require it.
================
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);
----------------
Is this covered by a test case?
================
Comment at: lib/Driver/ToolChain.cpp:827
+ options::OPT_fopenmp_targets_EQ).size() != 1) {
+ getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
+ continue;
----------------
Is this covered by a test case?
================
Comment at: lib/Driver/ToolChain.cpp:834
+ // Ignore all but last -march=arch flag.
+ if (A->getOption().matches(options::OPT_march_EQ))
+ DAL->eraseArg(options::OPT_march_EQ);
----------------
Why is `-march` special in this regard? Shouldn't the consumers just take the last one specified (e.g., use getLastArgValue in the ToolChain code)?
================
Comment at: test/Driver/openmp-offload.c:615
+
+// CHK-FOPENMP-TARGET: clang{{.*}} argument unused during compilation: '-Xopenmp-target -march=pwr8'
----------------
Now that this is in common code, why are these arguments still unused?
https://reviews.llvm.org/D34784
More information about the cfe-commits
mailing list