[PATCH] D21114: [DebugInfo] Add calling convention support for DWARF and CodeView

[Amjad Aboud via llvm-commits]

aaboud added a comment.

In general, direction looks good to me.
I add some minor comments below.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:2412
@@ -2411,3 +2411,3 @@
     case bitc::METADATA_SUBROUTINE_TYPE: {
-      if (Record.size() != 3)
+      if (Record.size() != 3 && Record.size() != 4)
         return error("Invalid record");
----------------
Consider emitting the CC as last field in the bitcode, i.e. Record[3].
Notice that the order in bitcode and text does not have to be the same.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1469
@@ -1468,2 +1468,3 @@
   Record.push_back(N->getFlags());
+  Record.push_back(N->getCC());
   Record.push_back(VE.getMetadataOrNullID(N->getTypeArray().get()));
----------------
Consider emitting CC as last field (see previous comment).

================
Comment at: lib/Support/Dwarf.cpp:392
@@ -391,3 @@
-   case DW_CC_nocall:                     return "DW_CC_nocall";
-   case DW_CC_lo_user:                    return "DW_CC_lo_user";
-   case DW_CC_hi_user:                    return "DW_CC_hi_user";
----------------
Notice that we had the DW_CC_lo_user and SW_CC_hi_user before, and now we do not.
Is that intentional?

================
Comment at: test/DebugInfo/X86/DW_AT_calling-convention.ll:1
@@ +1,2 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump %t | FileCheck %s
----------------
Do you have the original source of this test?
Can you add it as a comment into the LIT test?

================
Comment at: unittests/IR/MetadataTest.cpp:75
@@ -74,3 +74,3 @@
   MDNode *getNode() { return MDNode::get(Context, None); }
   MDNode *getNode(Metadata *MD) { return MDNode::get(Context, MD); }
   MDNode *getNode(Metadata *MD1, Metadata *MD2) {
----------------
How about adding a test case that checks setting CallingConvention correctly?


http://reviews.llvm.org/D21114