[PATCH] Emit Clang version information into .comment section (Clang's part of implementation) [PART 1]

Katya Romanova Katya_Romanova at playstation.sony.com
Mon Sep 30 08:28:33 PDT 2013


  Hi Rafael,

  Thank you so much for the review. I'm not sure I completely understand your question
  "Do you expect llvm to reason about this in any way?". If you mean whether llvm will use this information in any way, than the answer is "yes". Please refer to the Phabricator review http://llvm-reviews.chandlerc.com/D1729 "Emit Clang version information into .comment section (LLVM's part of implementation) [PART 2]" for information on how llvm is consuming the produced metadata. Also, in that review I discuss why emitting Clang's version info through inline asm with .ident is not such a good solution as emitting it as metadata. If you don't have time to read that review, let me briefly describe the reasons:

  (1) llvm.ident metadata could be used not only by LLVM, but by other consumers that read LLVM IR
   and want to generate a version information. It is easier for the LLVM IR consumers to understand the metadata than parse and understand inlined assembly.
  (2) llvm.ident metadata could be produced not only by Clang, but by other producers that generate LLVM IR. Again, producing the metadata is easier then producing inlined assembly.
  (3) It makes it easier to handle non-elf targets that don't support .ident.

  With respect of the changes that you suggested.
  - I modified the testcase to handle cases where CLANG_VENDOR is defined.
  - I'm hesitant to spell EmitVersionIdentMetadata() function name with lowercase. All other functions
  Emit<Something> in CodeGenModule.h and CodeGenModule.cpp are spelled with the first uppercase letter.
  That's the reason why I did the same. Otherwise, my function will really stand out among
  all other functions in these files. I think it better to do things consistently and commit "as is", but at some point of time to refactor and change the spelling of *all* of the function names in these files with lowercase.
  Let me know if you are OK with keeping the name of EmitVersionIdentMetadata() spelled with uppercase first letter.

  I will send an updated patch when I hear back from you.

http://llvm-reviews.chandlerc.com/D1720



More information about the cfe-commits mailing list