[PATCH] D123238: [NFC][CodeGen] Add comments for SDNode debug ID declaration

[Daniil Kovalev via Phabricator via llvm-commits]

kovdan01 created this revision.
kovdan01 added reviewers: dexonsmith, thakis.
Herald added a project: All.
kovdan01 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Normally, we place fields serving for debug purpose declarations
under `#if LLVM_ENABLE_ABI_BREAKING_CHECKS`. For `SDNode::PersistentId` and
`SelectionDAG::NextPersistentId`, we do not want to do so because it adds
unneeded complexity without noticeable benefits (see discussion with @thakis
in D120714 <https://reviews.llvm.org/D120714>). This patch adds comments describing why we don't place those
fields under `#if` not to confuse anyone more.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123238

Files:
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h


Index: llvm/include/llvm/CodeGen/SelectionDAGNodes.h
===================================================================
--- llvm/include/llvm/CodeGen/SelectionDAGNodes.h
+++ llvm/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -615,8 +615,10 @@
   SDNodeFlags Flags;
 
 public:
-  /// Unique and persistent id per SDNode in the DAG.
-  /// Used for debug printing.
+  /// Unique and persistent id per SDNode in the DAG. Used for debug printing.
+  /// We do not place that under `#if LLVM_ENABLE_ABI_BREAKING_CHECKS`
+  /// intentionally because it adds unneeded complexity without noticeable
+  /// benefits (see discussion with @thakis in D120714).
   uint16_t PersistentId;
 
   //===--------------------------------------------------------------------===//
Index: llvm/include/llvm/CodeGen/SelectionDAG.h
===================================================================
--- llvm/include/llvm/CodeGen/SelectionDAG.h
+++ llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -280,6 +280,11 @@
 
   DenseMap<const SDNode *, CallSiteDbgInfo> SDCallSiteDbgInfo;
 
+  /// PersistentId counter to be used when inserting the next
+  /// SDNode to this SelectionDAG. We do not place that under
+  /// `#if LLVM_ENABLE_ABI_BREAKING_CHECKS` intentionally because
+  /// it adds unneeded complexity without noticeable
+  /// benefits (see discussion with @thakis in D120714).
   uint16_t NextPersistentId = 0;
 
   /// Are instruction referencing variable locations desired for this function?


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123238.420948.patch
Type: text/x-patch
Size: 1475 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220406/eb77a3a3/attachment.bin>