[PATCH] D84099: [llvm-reduce] Fix off-by one in argument reduction pass

Alexander Richardson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 18 08:05:31 PDT 2020


arichardson marked an inline comment as done.
arichardson added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceArguments.cpp:84-87
     int ArgI = 0;
     for (auto &Arg : F->args())
       if (ArgsToKeep.count(&Arg))
+        ArgIndexesToKeep.insert(ArgI++);
----------------
lebedev.ri wrote:
> Let's take this one step further, shall we?
> ```
>     std::set<int> ArgIndexesToKeep;
>     for (auto &Arg : enumerate(F->args()))
>       if (ArgsToKeep.count(&Arg.value()))
>         ArgIndexesToKeep.insert(Arg.index());
> ```
Yes, much better! I realized just after submitting this patch that the index is not always being incremented so my non-minimal test case still crashed. Will add a better test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84099





More information about the llvm-commits mailing list