sfantao added a comment.

Hi Hal,

Thanks for the review! Comments inlined.



================
Comment at: lib/Driver/Tools.cpp:334
+  LksStream << "  OpenMP Offload Linker Script.\n";
+  LksStream << "*/\n";
+  LksStream << "TARGET(binary)\n";
----------------
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 ***
```


================
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;
----------------
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?


https://reviews.llvm.org/D21847



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to