[PATCH] D107845: [DAG] Reword comment for EnforceNodeIdInvariant and InvalidateNodeId. NFC.

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 10 10:44:10 PDT 2021


craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1057
+// selection we may induce a predecessor relationship between inputs and
+// outputs of distinct nodes being merged violating the topological
+// property. Should a fused node have a successor which has yet to be selected,
----------------
I think there should be a comma after "merged".


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1058
+// outputs of distinct nodes being merged violating the topological
+// property. Should a fused node have a successor which has yet to be selected,
+// our legality checks would be incorrect. To avoid this we mark all unselected
----------------
Why did "property." get bumped off the previous line?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1060
+// our legality checks would be incorrect. To avoid this we mark all unselected
 // sucessor nodes, i.e. id != -1 as invalid for pruning by bit-negating (x =>
 // (-(x+1))) the ids and modify our pruning check to ignore negative Ids of M.
----------------
niravd wrote:
> While we're at it, "successor"*
comma after -1 I think.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:1063
 // We use bit-negation to more clearly enforce that node id -1 can only be
-// achieved by selected nodes). As the conversion is reversable the original Id,
+// achieved by selected nodes. As the conversion is reversable the original Id,
 // topological pruning can still be leveraged when looking for unselected nodes.
----------------
"reversable the original Id" -> "reversable to the original Id" maybe?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107845/new/

https://reviews.llvm.org/D107845



More information about the llvm-commits mailing list