[PATCH] D30040: [RewriteStatepointsForGC] Removes some unnecessary calls for operator[].

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

All of the const changes can be separate and submitted.  (Please do so before posting revised patch to simplify review.)



================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:762
       Value *BDV = Pair.first;
+      BDVState &State = Pair.second;
       assert(!isKnownBaseResult(BDV) && "why did it get added?");
----------------
Sink variable to use please.

Also, assigning to reference is mildly confusing.  Can you add a comment to clarify?


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:819
     Instruction *I = cast<Instruction>(Pair.first);
-    BDVState State = Pair.second;
+    BDVState &State = Pair.second;
     assert(!isKnownBaseResult(I) && "why did it get added?");
----------------
This appears to be changing behaviour.  The State variable previous survive to end of scope unmodified and is now modified part way through before hitting more conditional code.  Please revert.


https://reviews.llvm.org/D30040