[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.


More information about the llvm-commits mailing list