[PATCH] Move easily derivable pointers optimization from CodeGenPrepare to RewriteStatepointPass

[Igor Laevsky]

REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1456
@@ +1455,3 @@
+                                        F.getEntryBlock().getFirstNonPHI());
+    allocaMap[LiveValue] = Alloca;
+    PromotableAllocas.push_back(Alloca);
----------------
sanjoy wrote:
> Variable naming in this function is somewhat haphazard.  Do you mind bringing the variable names up to LLVM's coding standards for the functions you touched in a later separate NFC change, (once this change is in)?
Sure

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1873
@@ +1872,3 @@
+
+  // Avoid rematerializing very long instruction chains to avoid code size
+  // problems
----------------
sanjoy wrote:
> What is a situation where this guard will help?
In theory TTI functions can return zero cost. In that case it is possible to have very long instruction chain with estimated cost of zero. It does not seems very right. This is purely hypothetical and I don't think it is  very likely to happen on practice.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1914
@@ +1913,3 @@
+    }
+    // If it's too expensive - skip it
+    if (Cost >= RematerializationThreshold)
----------------
sanjoy wrote:
> No need to fix this now, but add a `TODO` for adjusting the cost when recomputing the derived pointer will let you remove the computation of the unrelocated derived pointer in cases where the only use of the derived pointer was the statepoint.
Not sure if I am following.

Are you saying about cases when we will be able to remove instruction chain (or part of it) later? I.e it also can happen if there was several intersecting instruction chains we have rematerialized.

================
Comment at: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:1934
@@ +1933,3 @@
+        // Only GEP's and casts are suported as we need to be careful to not
+        // introduce any new uses of pointers not in the liveset.
+        assert(isa<GetElementPtrInst>(Instr) || isa<CastInst>(Instr));
----------------
sanjoy wrote:
> But you may still semantically "promote" a live value that was solely the base pointer for another derived pointer (and had no direct uses after the safepoint) to a pointer that is fundamentally live over the safepoint (has a direct use after the safepoint), right?  Will that always work correctly?
Yes, it should work. Nowhere in this pass we distinguish pointers which are live "for real" and base pointer which are required to be live because of the derived pointer, but does not have uses of their own.

http://reviews.llvm.org/D9774

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