[PATCH] D50094: Introduce DebugCounter into ConstProp pass
George Burgess IV via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 1 13:32:01 PDT 2018
george.burgess.iv added inline comments.
================
Comment at: lib/Transforms/Scalar/ConstantProp.cpp:88
+ // Iterate element from the worklist in stable order
+ for (Instruction *I: WorkListVec) {
+ WorkList.erase(I); // Remove element from the worklist...
----------------
george.burgess.iv wrote:
> We can't do a range for loop here: they expand to (roughly; I'm too lazy to bring up the standard, and the differences don't matter here :) )
>
> ```
> for (auto Begin = WorkListVec.begin(), End = WorkListVec.end(); Begin != End; ++Begin) {
> Instruction *I = *Begin;
> {
> // Loop body
> }
> }
> ```
>
> This is problematic, since `WorkListVec.push_back()` invalidates iterators, but we're still using the now-invalid iterators.
>
> Please either make this use indices, or a `while (!vec.empty()) { auto *i = vec.pop_back_val(); }`-like thing
I think there was a misunderstanding here. Please use an index instead of an iterator. The loop should look like
```
for (unsigned I = 0; I < WorkListVec.size(); ++I) {
...
}
```
Repository:
rL LLVM
https://reviews.llvm.org/D50094
More information about the llvm-commits
mailing list