[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