[PATCH] D13491: [RS4GC] Refactoring to make a later change easier, NFCI

[Sanjoy Das via llvm-commits]

sanjoy marked 2 inline comments as done.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1357
@@ +1356,3 @@
+  ArrayRef<Value *> GCArgs(LiveVariables);
+  uint64_t StatepointID = SP.getID();
+  uint32_t NumPatchBytes = SP.getNumPatchBytes();
----------------
reames wrote:
> const?
I'd then have to remove the `const` in the later semantic patch, which would make that diff larger.

================
Comment at: test/Transforms/RewriteStatepointsForGC/relocation.ll:97
@@ -96,3 +96,3 @@
 ; CHECK-DAG: [ %arg.relocated, %if_branch ]
-; CHECK-DAG: [ %arg.relocated4, %else_branch ]
+; CHECK-DAG: [ %arg.relocated3, %else_branch ]
 ; CHECK-NOT: phi
----------------
reames wrote:
> I'm not seeing why this changed?  Is this a non-determinism?  Or did something in your patch effect ordering?
It looks like there is a "semantic" change in this refactoring.  I'm
not a 100% confident that I understand what's going on, but it looks
like `CreateGCStatepointCall` creates a named call instruction and
//then// inserts the said call instruction, whereas `CreateCall`
creates a nameless instruction, inserts it, and then calls `setName`
on it.  This changes the number of times `LastUnique` is incremented
for the `Function` s `ValueSymbolTable` because calling `setName` on
an unattached instruction does not update the symbol table (since
there is no symbol table to update).

This does not really matter though -- only the stringified name is
affected, the bits of IR that matter should remain the same.


http://reviews.llvm.org/D13491