[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