Emit Clang version information into .comment section (CodeGen's part of implementation) [PART 2A]

[kromanova]

Summary

This is a spin-off of a review Emit Clang version information into .comment section (LLVM's part of implementation) [PART 2]
http://llvm-reviews.chandlerc.com/D1729

The reviewer, Rafael Espíndola, asked to split LLVM's part of the patch into two separate patches (CodeGen and MC). Here is the CodeGen part of the patch with the few corrections.

[• rafael]

You are also missing a test that the verifier rejects broken IL files.

What is the URL for the MC part?

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1347

please use cast instead of dyn_cast and have the verifier check that we only get strings in here.

lib/IR/Verifier.cpp

628

Please check that it is a string too.

[kromanova]

Fixed few things that Rafael suggested.

[• rafael]

Almost there, just a few nits.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1347

You don't need the if when using cast. It return non-null or asserts.

lib/IR/Verifier.cpp

621

Please put the return in the next line.

clang-formating the patch would be a good idea.

test/CodeGen/X86/ident-metadata.ll

3

No need for the -filetype=obj test. I will be tested in the MC part.

[kromanova]

Few more minor corrections are done based on the feedback.
I was wondering what is the common set of options to pass to clang-format for formatting the patch to comply with the Clang/LLVM style?

Thanks!

[• rafael]

LGTM. Do you need me to commit this or do you have commit access?

[kromanova]

Hi Rafael,
I don't have a commit access. Please don't commit yet, since MC part needs to be committed first (otherwise we will break the build). I am working on MC part changes now.
Katya.

[Eugene.Zelenko]

Committed in rL192764.

include/llvm/CodeGen/AsmPrinter.h

                             const MachineBasicBlock *MBB,
                             unsigned uid) const;
     void EmitLLVMUsedList(const ConstantArray *InitList);
+    /// Emit llvm.ident metadata in an '.ident' directive.
+    void EmitModuleIdents(Module &M);
     void EmitXXStructorList(const Constant *List, bool isCtor);
     GCMetadataPrinter *GetOrCreateGCPrinter(GCStrategy *C);
   };

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

     if (GCMetadataPrinter *MP = GetOrCreateGCPrinter(*--I))
       MP->finishAssembly(*this);
 
+  // Emit llvm.ident metadata in an '.ident' directive.
+  EmitModuleIdents(M);
+
   // If we don't have any trampolines, then we don't require stack memory
   // to be executable. Some targets have a directive to declare this.
   Function *InitTrampolineIntrinsic = M.getFunction("llvm.init.trampoline");

   }
 }
 
+void AsmPrinter::EmitModuleIdents(Module &M) {
+  if (!MAI->hasIdentDirective())
+    return;
+
+  if (const NamedMDNode *NMD = M.getNamedMetadata("llvm.ident")) {
+    for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
+      const MDNode *N = NMD->getOperand(i);
+      assert(N->getNumOperands() == 1 && 
+             "llvm.ident metadata entry can have only one operand");
+      const MDString *S = cast<MDString>(N->getOperand(0));
+      OutStreamer.EmitIdent(S->getString());
+    }
+  }
+}
+
 //===--------------------------------------------------------------------===//
 // Emission and print routines
 //

lib/IR/Verifier.cpp

         visitNamedMDNode(*I);
 
       visitModuleFlags(M);
+      visitModuleIdents(M);
 
       // Verify Debug Info.
       verifyDebugInfo(M);

     void visitGlobalAlias(GlobalAlias &GA);
     void visitNamedMDNode(NamedMDNode &NMD);
     void visitMDNode(MDNode &MD, Function *F);
+    void visitModuleIdents(Module &M);
     void visitModuleFlags(Module &M);
     void visitModuleFlag(MDNode *Op, DenseMap<MDString*, MDNode*> &SeenIDs,
                          SmallVectorImpl<MDNode*> &Requirements);

   }
 }
 
+void Verifier::visitModuleIdents(Module &M) {
+  const NamedMDNode *Idents = M.getNamedMetadata("llvm.ident");
+  if (!Idents) 
+    return;
+  
+  // llvm.ident takes a list of metadata entry. Each entry has only one string.
+  // Scan each llvm.ident entry and make sure that this requirement is met.
+  for (unsigned i = 0, e = Idents->getNumOperands(); i != e; ++i) {
+    const MDNode *N = Idents->getOperand(i);
+    Assert1(N->getNumOperands() == 1,
+            "incorrect number of operands in llvm.ident metadata", N);
+    Assert1(isa<MDString>(N->getOperand(0)),
+            ("invalid value for llvm.ident metadata entry operand"
+             "(the operand should be a string)"),
+            N->getOperand(0));
+  } 
+}
+
 void Verifier::visitModuleFlags(Module &M) {
   const NamedMDNode *Flags = M.getModuleFlagsMetadata();
   if (!Flags) return;

test/CodeGen/X86/ident-metadata.ll

+; RUN: llc -march=x86 < %s | FileCheck %s
+; Verify that llvm.ident metadata is emitted as .ident
+; directives in assembly files, and in the .comment section in ELF object files.
+
+; CHECK: .ident  "clang version x.x"
+; CHECK-NEXT: .ident  "something else"
+!llvm.ident = !{!0, !1}
+!0 = metadata !{metadata !"clang version x.x"}
+!1 = metadata !{metadata !"something else"}

test/Verifier/ident-meta1.ll

+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+; Verify that llvm.ident is properly structured.
+; llvm.ident takes a list of metadata entries.
+; Each metadata entry can have only one string.
+
+!llvm.ident = !{!0, !1}
+!0 = metadata !{metadata !"version string"}
+!1 = metadata !{metadata !"string1", metadata !"string2"}
+; CHECK: assembly parsed, but does not verify as correct!
+; CHECK-NEXT: incorrect number of operands in llvm.ident metadata
+; CHECK-NEXT: metadata !1
+

test/Verifier/ident-meta2.ll

+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+; Verify that llvm.ident is properly structured.
+; llvm.ident takes a list of metadata entries.
+; Each metadata entry can contain one string only.
+
+!llvm.ident = !{!0, !1, !2, !3}
+!0 = metadata !{metadata !"str1"}
+!1 = metadata !{metadata !"str2"}
+!2 = metadata !{metadata !"str3"}
+!3 = metadata !{i32 1}
+; CHECK: assembly parsed, but does not verify as correct!
+; CHECK-NEXT: invalid value for llvm.ident metadata entry operand(the operand should be a string)
+; CHECK-NEXT: i32 1

test/Verifier/ident-meta3.ll

+; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s
+; Verify that llvm.ident is properly structured.
+; llvm.ident takes a list of metadata entries.
+; Each metadata entry can contain one string only.
+
+!llvm.ident = !{!0}
+!0 = metadata !{metadata !{metadata !"nested metadata"}}
+; CHECK: assembly parsed, but does not verify as correct!
+; CHECK-NEXT: invalid value for llvm.ident metadata entry operand(the operand should be a string)
+; CHECK-NEXT: metadata !1