[PATCH] D24399: [RS4GC] Avoid usage of unrelocated values while rematerializing geps and casts

[Philip Reames via llvm-commits]

reames added a comment.

Looks close to ready, one more update should do it.


================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1983
@@ -1980,1 +1982,3 @@
+        Instruction *InsertBefore, Value *RootOfChain, Value *AlternateLiveBase,
+        bool &ReplaceUnrelocatedUse) {
       Instruction *LastClonedValue = nullptr;
----------------
ReplaceUnrelocatedUse doesn't need to be a reference.  And shouldn't be.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2006
@@ -2001,3 +2005,3 @@
           for (auto OpValue : ClonedValue->operand_values()) {
             assert(!is_contained(ChainToBase, OpValue) &&
                    "incorrect use in rematerialization chain");
----------------
Strengthen this assert to include the root value and the new root value.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:2016
@@ +2015,3 @@
+          // ChainToBase list.
+          if (ReplaceUnrelocatedUse)
+            ClonedValue->replaceUsesOfWith(RootOfChain, AlternateLiveBase);
----------------
I don't think you actually need this variable.  If you just check OldRoot != NewRoot, doesn't that give you the same property?


https://reviews.llvm.org/D24399