[PATCH] D143323: [ConstraintElim] Add reproducer remarks.

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 13 10:12:30 PST 2023


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

Only things I can think of that would be nice to add are

- Some debug output, if possible
- Comments for the lambdas + some of the loops

Other than that, this LGTM.



================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:807
+  SmallPtrSet<Value *, 8> Seen;
+  auto CollectArguments = [&](CmpInst *Cond) {
+    if (!Cond)
----------------
Could this have a comment that outlines which part it plays in the overall algorithm?


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:849
+                                 M);
+  for (unsigned I = 0; I < Args.size(); ++I) {
+    F->getArg(I)->setName(Args[I]->getName());
----------------
A comment here would be useful for future readers as well.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:859
+
+  auto CloneInstructions = [&](ArrayRef<Value *> Ops, bool IsSigned) {
+    SmallVector<Value *, 4> WorkList(Ops);
----------------
This can use a comment too (even though the name is pretty descriptive, it does some other stuff such as sorting the instructions)


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:886
+  };
+  for (auto &Entry : Stack) {
+    if (!Entry.Cond)
----------------
Comment on this loop too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143323



More information about the llvm-commits mailing list