[PATCH] Support of the gc.relocates for invoke statepoints

[Igor Laevsky]

REPOSITORY
  rL LLVM

================
Comment at: include/llvm/CodeGen/FunctionLoweringInfo.h:82
@@ +81,3 @@
+  // for consistency checks.
+  DenseMap<const Value*, std::pair<bool, int>> InvokeStatepointValueSlots;
+  
----------------
reames wrote:
> I don't think we're actually guaranteed that the same stack slot is used across all statepoints.  Are we?  If not, this needs to be keyed by a pair of derived pointer and statepoint instruction.
> 
> Consider:
> V = ...
> if (X) statepoint; else statepoint;
You are right, we are not guaranteed that same stack slot is allocated for the same value in different statepoints. But we are guaranteed that when we start processing new safepoint with same value we already finished processing all gc relocates of the previous one and can safely discard previously saved location. 

See comment on line 557 in StatepointLowering.cpp for more info. Test case "test_same_val" checks this case.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:535
@@ +534,3 @@
+  if (StatepointSite.getCallSite().isInvoke()) {
+    for (GCRelocateOperands relocateOpers :
+           StatepointSite.getRelocates(StatepointSite)) {
----------------
reames wrote:
> Given we already have the derived pointers above, can't this be rewritten as a loop over Ptrs?
> 
> Actually, I would have expected to see this as part of the previous loop.  If lowerIncomingStatepointValue returned the lowered value, this code could be expressed neatly within the previously loop.
In Ptr's array we only have values with unique SDValues. But we need to store locations of the all possible relocated values. Notice that this loop is over StatepointSite.getRelocates().


================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:746
@@ +745,3 @@
+  if ((relocateOpers.isTiedToInvoke() &&
+       !FuncInfo.InvokeStatepointValueSlots[DerivedPtr].first)) {
+    // In this case we have visited invoke statepoint value
----------------
reames wrote:
> Er, I don't think there should be such a case.  Do you have an example or should this be an assert?
There is such cases for constants, allocas and undefs. See "test_null_undef" and "test_alloca_and_const" tests. 

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:755
@@ -701,2 +754,3 @@
+
   SDValue Loc = StatepointLowering.getRelocLocation(SD);
   // Emit new load if we did not emit it before
----------------
reames wrote:
> This really doesn't look like right.  The allocation logic used inside spilling is assuming it can reuse slots.  It looks like you've essentially created an alternate mechanism for recording locations in invokes, but not updated the stack slot reuse/allocation code paths.  Am I missing something here?
At the beginning of each statepoint we know that all previous relocates were visited (otherwise statepoint intervals will intersect and it does not have any sense). It means that we are not going to need locations for this relocates and can reuse all allocated slots.

To be clear, by "previous relocates" I mean relocates that dominate current safepoint. For example relocates on exceptional path could have been left unvisited. This is not problematic because we are still reusing slots only at the point dominated by only visited relocates. I.e when we start reusing slots on exceptional path all exceptional relocates should be visited.

http://reviews.llvm.org/D7798

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/