hfinkel accepted this revision. hfinkel added a comment. This revision is now accepted and ready to land.
LGTM ================ Comment at: lib/Driver/Tools.cpp:334 + LksStream << " OpenMP Offload Linker Script.\n"; + LksStream << "*/\n"; + LksStream << "TARGET(binary)\n"; ---------------- sfantao wrote: > hfinkel wrote: > > We should also say 'autogenerated' somewhere in this comment. > Ok, makes sense. The comment is now: > ``` > OpenMP Offload Linker Script. > *** Automatically generated by clang *** > ``` Please capitalize Clang (it is capitalized in our documentation). ================ Comment at: lib/Driver/Tools.cpp:386 + // Dump the contents of the linker script if the user requested that. + if (C.getArgs().hasArg(options::OPT_fopenmp_dump_offload_linker_script)) + llvm::errs() << LksBuffer; ---------------- sfantao wrote: > hfinkel wrote: > > I don't see why this is needed if we have -save-temps - I think we should > > remove this option entirely. > The reason for adding this option is that the test is done when the driver is > in dry-run mode (`-###`) so I'm not supposed to generate any files. If we > don't run in dry-run mode, we need to allow linking to actually happen, > therefore the machine where the tests runs needs to have a gcc-based > toolchain and ld. > > Is there a way to request that in the required features set in llvm-lit > config file? Should I add a new feature? Okay, I understand. Please add to the comment that this is used so that we can test test behavior with -###. I don't think that we want the regression tests to really call the system linker, etc. https://reviews.llvm.org/D21847 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
