[PATCH] D45341: [DebugInfo] Convert intrinsic llvm.dbg.label to MachineInstr.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Okay, let's do a separate machine instruction then!



================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.cpp:754
 
+/// EmitDbgLabel - Generate machine instruction for a dbg_label node.
+///
----------------
Again, the comment should be on the declaration, not the definition.


================
Comment at: lib/CodeGen/SelectionDAG/InstrEmitter.h:116
 
+  /// EmitDbgLabel - Generate machine instruction for a dbg_label node.
+  ///
----------------
don't repeat the name in the comment


================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:118
 
+/// SDDbgLabel - Holds the information from a dbg_label node through SDISel.
+/// We do not use SDValue here to avoid including its header.
----------------
Please don't repeat the class name in comments.


================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:120
+/// We do not use SDValue here to avoid including its header.
+
+class SDDbgLabel {
----------------
extra newline


================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:130
+
+  // Returns the MDNode pointer for the label.
+  MDNode *getLabel() const { return Label; }
----------------
///


================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:133
+
+  // Returns the DebugLoc.
+  DebugLoc getDebugLoc() const { return DL; }
----------------
///


================
Comment at: lib/CodeGen/SelectionDAG/SDNodeDbgValue.h:136
+
+  // Returns the SDNodeOrder.  This is the order of the preceding node in the
+  // input.
----------------
///


================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:918
+    LastOrder = 0;
+    for (unsigned i = 0, e = Orders.size(); i != e && DLI != DLE; ++i) {
+      unsigned Order = Orders[i].first;
----------------
perhaps use a range-based for?


================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:931
+            // Insert to start of the BB (after PHIs).
+            BB->insert(BBBegin, DbgMI);
+          else {
----------------
Do you have test coverage for both paths?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7779
 
+/// AddDbgLabel - Add a dbg_label SDNode.
+void SelectionDAG::AddDbgLabel(SDDbgLabel *DB) {
----------------
The comment should be on the function *declaration*, and not repeat the function name:
`/// Add a dbg_label SDNode.`

http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments


================
Comment at: test/DebugInfo/Generic/debug-label-mi.ll:5
+;
+; CHECKMI: DBG_LABEL "top"
+; CHECKMI: DBG_LABEL "done"
----------------
can you also check the !dbg attachment?


Repository:
  rL LLVM

https://reviews.llvm.org/D45341