[PATCH] D72125: [NFC] Move InPQueue into normal argument in releaseNode

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 5 07:09:29 PST 2020


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, but it would be great to document the releaseNode function, including the parameters :)



================
Comment at: llvm/include/llvm/CodeGen/MachineScheduler.h:760
 
-  template <bool InPQueue>
-  void releaseNode(SUnit *SU, unsigned ReadyCycle, unsigned Idx = 0);
+  void releaseNode(SUnit *SU, unsigned ReadyCycle, bool InPQueue,
+                   unsigned Idx = 0);
----------------
It would be great if you could add a brief doc comment here, stating what the arguments mean.


================
Comment at: llvm/include/llvm/CodeGen/MachineScheduler.h:959
 
-    Top.releaseNode<false>(SU, SU->TopReadyCycle);
+    Top.releaseNode(SU, SU->TopReadyCycle, false);
     TopCand.SU = nullptr;
----------------
DoktorC wrote:
> Would it be worth to add a `/*InPQueue=*/` comment next to the new argument?
IIRC the comments are only used for arguments with default values. I don't think it is necessary here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72125





More information about the llvm-commits mailing list