[PATCH] D31124: AMDGPU/SI: Add lane tracking to SI Scheduler

[Valery Pykhtin via Phabricator via llvm-commits]

vpykhtin added a comment.

For the second assert it may be the case for iterator invalidation, though I haven't checked.



================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:210
+        *I = Elem & Remaining;
+        LaneBasis.push_back(NewElem);
+      }
----------------
axeldavy wrote:
> vpykhtin wrote:
> > You're iterating over a vector increasing its' size at the same time. This is almost ok, except that vector can reallocate and all iterators would become invalidated. 
> > 
> > Iterator Invalidation Rules (C++03)
> > vector: all iterators and references before the point of insertion are unaffected, unless the new container size is greater than the previous capacity (in which case all iterators and references are invalidated) [23.2.4.3/1]
> What do you suggest to fix this problem ?
Easy way is to reserve sufficient number of elements to avoid reallocation, but this is not reliable as code correctness depends on reserve which is in general optional. You may consider using std::list for this.


================
Comment at: lib/Target/AMDGPU/SIMachineScheduler.cpp:376
+
+#ifndef NDEBUG
+
----------------
axeldavy wrote:
> vpykhtin wrote:
> > #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> > LLVM_DUMP_METHOD
> Why LLVM_DUMP_METHOD on an empty line ?
I meant something like this:

#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

LLVM_DUMP_METHOD 
SISchedulerRPTracker::printDebugLives()


Repository:
  rL LLVM

https://reviews.llvm.org/D31124