[PATCH] [StatepointLowering] Reuse stack slots across basic blocks

Sanjoy Das sanjoy at playingwithpointers.com
Mon Jun 8 23:47:12 PDT 2015


LGTM with comments.


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:135
@@ +134,3 @@
+    if (It == SpillMap.end())
+      return Optional<int>();
+
----------------
When does this happen (derived pointer is not in SpillMap)?

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:141
@@ +140,3 @@
+  // Look through cast instructions. Most of them are nops.
+  if (const CastInst *Cast = dyn_cast<CastInst>(Val)) {
+    return findPreviousSpillSlot(Cast->getOperand(0), Builder, LookUpDepth - 1);
----------------
The `{` is redundant here.

Also, is it okay to look through `addrspacecast`s?  Perhaps we should just look through `bitcast`s for now and later generalize if needed.

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:171
@@ +170,3 @@
+  // llvm's ability to remove redundant stores.
+  // Unfortunatelly it's hard to accomplish in current infrastructure.
+  // We use this function to eliminate spill store complitelly, while
----------------
Nit: `Unfortunately`

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:172
@@ +171,3 @@
+  // Unfortunatelly it's hard to accomplish in current infrastructure.
+  // We use this function to eliminate spill store complitelly, while
+  // in example we still need to emit store, but instead of any location
----------------
Nit: `completely`

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:183
@@ +182,3 @@
+  //   statepoint (i1)
+  // However we need to be carefull for cases like this:
+  //   statepoint(i)
----------------
Nit: `careful`

================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:524
@@ -470,2 +523,3 @@
   // reserve slots for both deopt and gc values before lowering either.
   for (const Value *V : StatepointSite.vm_state_args()) {
+    reservePreviousStackSlotForValue(V, Builder);
----------------
The braces are now unnecessary.

http://reviews.llvm.org/D10251

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






More information about the llvm-commits mailing list