[PATCH] D21427: NFC; refactor getFrameIndexReferenceFromSP

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 16 08:20:30 PDT 2016


rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm with the assert


================
Comment at: include/llvm/Target/TargetFrameLowering.h:257
@@ +256,3 @@
+                                             unsigned &FrameReg,
+                                             bool AllowSPAdjustment) const {
+    // Always safe to dispatch to getFrameIndexReference.
----------------
Can we rename "AllowSPAdjustment" to something better? As the doc comments say, it's really about getting the offset relative to SP at end of prologue. A better name might be IgnoreSPUpdates or IgnoreSPAdjustments or OffsetFromInitialSP or OffsetFromSPAfterPrologue.

================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:307
@@ -307,1 +306,3 @@
+                                              /*AllowSPAdjustment*/ true);
+
   // For 32-bit, offsets should be relative to the end of the EH registration
----------------
We could try this assert:
  assert(UnusedReg == Asm->MF->getSubtarget().getTargetLowering()->getStackPointerRegisterToSaveRestore());
Beautiful, I know.

The only way this can end up using the FP and we fail the assert is if we try to reference a fixed object with stack realignment. In this code, FrameIndex should never refer to a fixed stack object, so we should be OK.


http://reviews.llvm.org/D21427





More information about the llvm-commits mailing list