Support `#pragma comment(lib, "name")` in the frontend for ELF

[compnerd]

Summary

This adds the frontend support required to support the use of the comment pragma to enable auto linking on ELFish targets. This is a generic ELF extension supported by LLVM. We need to change the handling for the "dependentlib" in order to accommodate the previously discussed encoding for the dependent library descriptor. Without the custom handling of the PCK_Lib directive, the -l prefixed option would be encoded into the resulting object (which is treated as a frontend error).

[compnerd]

Add missed file

[ruiu]

#pragma comment is what Microsoft is using, but is everybody happy about that name? It isn't clear at least to me that what it actually means is linker directives.

I also wonder which is better #pragma comment(lib, "m") or #pragma comment(lib, "m").

[ruiu]

I also wonder which is better #pragma comment(lib, "m") or #pragma comment(lib, "m").

Sorry, I meant #pragma comment(lib, "m") or #pragma comment("lib", "m").

[probinson]

In D42758#993936, @ruiu wrote:

I also wonder which is better #pragma comment(lib, "m") or #pragma comment(lib, "m").

Sorry, I meant #pragma comment(lib, "m") or #pragma comment("lib", "m").

I can't swear to it but I don't think Microsoft invented #pragma comment. Various IBM compilers have it, with a syntax of #pragma comment ( <keyword> [ , "string" ] ). I'm not seeing a lib keyword specifically in the IBM docs, but being a keyword would be consistent with past practice.

lib/Parse/ParsePragma.cpp

299

PS4's binary format is ELF, so you should be able to remove the isPS4 predicate.

382

Don't need isPS4, as it uses ELF.

[compnerd]

@probinson it would be pretty cool if we could get the PS4 environment to share the same linker options implementation. What other options do you guys need for this to be a viable approach?

[erichkeane]

This seems pretty OK to me. I'd like to see @probinson s PS4 discussion bottom out, but I don't see any reason to hold this up otherwise.

We definitely should leave the format as #pragma comment(keyword, "message"), since there is significant prior art here.

test/CodeGen/elf-linker-options.c

3 ↗

(On Diff #132252)

I would like a test for Sema to validate the 'pragma ignored' warning in common cases (such as 'Linker').

[compnerd]

clang-format missed line, simplify some checks

[majnemer]

docs/LanguageExtensions.html should be updated to mention that we support this extension on all ELF targets.

[jhenderson]

In D42758#997880, @erichkeane wrote:

I'd like to see @probinson s PS4 discussion bottom out, but I don't see any reason to hold this up otherwise.

The PS4 compiler uses its linker options mechanism to communicate three different things to the linker, namely #pragma comment(lib, ...) directives, as discussed here, symbols marked with __declspec(dllexport), and symbols marked with __declspec(dllimport) (the latter two requesting the linker explicitly to export/import the decorated symbols). I think if all three of these are converted to linker commands, we should be able to adopt the new implementation.

[compnerd]

@jhenderson I believe that the first one is what this is implementing. I believe that adding the last two as a patch following this one is preferable as that is specific to the needs for PS4, but, both of those should be possible to accommodate. I would love to see a single unified approach here, so Im happy to help get that implemented.

[compnerd]

Add additional test, update docs

[erichkeane]

Just 1 format question, otherwise Looks good.

docs/LanguageExtensions.rst

2732 ↗

(On Diff #133030)

The newlines in this section are seemingly inconsistent with what is above. Does that one violate the 80 char rule, or is this just short?

[compnerd]

docs/LanguageExtensions.rst

2732 ↗

(On Diff #133030)

Yeah, the previous section has 80-column violations :-(.

[compnerd]

SVN r324438

lib/CodeGen/CodeGenModule.h

   /// value.
   void AddDependentLib(StringRef Lib);
 
+  void AddELFLibDirective(StringRef Lib);
+
   llvm::GlobalVariable::LinkageTypes getFunctionLinkage(GlobalDecl GD);
 
   void setFunctionLinkage(GlobalDecl GD, llvm::Function *F) {

lib/CodeGen/CodeGenModule.cpp

   LinkerOptionsMetadata.push_back(llvm::MDNode::get(getLLVMContext(), MDOpts));
 }
 
+void CodeGenModule::AddELFLibDirective(StringRef Lib) {
+  auto &C = getLLVMContext();
+  LinkerOptionsMetadata.push_back(llvm::MDNode::get(
+      C, {llvm::MDString::get(C, "lib"), llvm::MDString::get(C, Lib)}));
+}
+
 void CodeGenModule::AddDependentLib(StringRef Lib) {
   llvm::SmallString<24> Opt;
   getTargetCodeGenInfo().getDependentLibraryOption(Lib, Opt);

       AppendLinkerOptions(PCD->getArg());
       break;
     case PCK_Lib:
-      AddDependentLib(PCD->getArg());
+      if (getTarget().getTriple().isOSBinFormatELF())
+        AddELFLibDirective(PCD->getArg());
+      else
+        AddDependentLib(PCD->getArg());
       break;
     case PCK_Compiler:
     case PCK_ExeStr:

lib/Parse/ParsePragma.cpp

     OpenMPHandler.reset(new PragmaNoOpenMPHandler());
   PP.AddPragmaHandler(OpenMPHandler.get());
 
-  if (getLangOpts().MicrosoftExt || getTargetInfo().getTriple().isPS4()) {
+  if (getLangOpts().MicrosoftExt || getTargetInfo().getTriple().isPS4() ||
+      getTargetInfo().getTriple().isOSBinFormatELF()) {
     MSCommentHandler.reset(new PragmaCommentHandler(Actions));
     PP.AddPragmaHandler(MSCommentHandler.get());
   }

   PP.RemovePragmaHandler(OpenMPHandler.get());
   OpenMPHandler.reset();
 
-  if (getLangOpts().MicrosoftExt || getTargetInfo().getTriple().isPS4()) {
+  if (getLangOpts().MicrosoftExt || getTargetInfo().getTriple().isPS4() ||
+      getTargetInfo().getTriple().isOSBinFormatELF()) {
     PP.RemovePragmaHandler(MSCommentHandler.get());
     MSCommentHandler.reset();
   }

     return;
   }
 
+  if (PP.getTargetInfo().getTriple().isOSBinFormatELF() && Kind != PCK_Lib) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_comment_ignored)
+      << II->getName();
+    return;
+  }
+
   // On PS4, issue a warning about any pragma comments other than
   // #pragma comment lib.
   if (PP.getTargetInfo().getTriple().isPS4() && Kind != PCK_Lib) {