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

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 6 10:52:21 PST 2023


paquette added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:778
 
-static bool checkAndReplaceCondition(CmpInst *Cmp, ConstraintInfo &Info) {
+struct ReproducerEntry {
+  CmpInst *Cond;
----------------
doxygen comments for this?


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:791
+static void generateReproducer(CmpInst *Cond, Module *M,
+                               ArrayRef<ReproducerEntry> Stack,
+                               DenseMap<Value *, unsigned> &Value2Index,
----------------
Can you document this parameter?


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:803
+  bool IsSigned = CmpInst::isSigned(Cond->getPredicate());
+  auto CollectArguments = [&](CmpInst *Cond) {
+    if (!Cond || IsSigned != CmpInst::isSigned(Cond->getPredicate()))
----------------
This lambda is long enough that I feel like it ought to just be a function (if possible)


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:846
+
+  for (unsigned i = 0; i < Args.size(); i++) {
+    Old2New[Args[i]] = F->getArg(i);
----------------
These loops can be merged into one loop


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