[PATCH] D11160: Targets: commonize some stack realignment code

Reid Kleckner rnk at google.com
Tue Jul 14 14:55:12 PDT 2015


rnk added a subscriber: rnk.
rnk added a comment.

This is only a breaking change in the sense that it affects `needsStackRealignment()` for out of tree targets, right? If not, can you elaborate on what I'm missing?

This looks like a good direction.


================
Comment at: lib/CodeGen/TargetRegisterInfo.cpp:315
@@ +314,3 @@
+                              F->hasFnAttribute(Attribute::StackAlignment));
+  if (requiresRealignment) {
+    if (canRealignStack(MF))
----------------
Did the assertion not make it past the test suite? It looks like you added the dbgs() after this comment.

================
Comment at: lib/Target/X86/X86RegisterInfo.cpp:473
@@ -472,3 +472,3 @@
   // it.
   if (MFI->hasVarSizedObjects())
     return MRI->canReserveReg(BasePtr);
----------------
This (and other) hasVarSizedObjects() checks should probably be looking for `MFI->hasVarSizedObjects() || MFI->hasOpaqueSPAdjustment()` as is done above in hasBasePointer. It catches the case where there is some other reason why SP is useless for addressing local variables, like SP-adjusting inline assembly or (ugh) SEH.


http://reviews.llvm.org/D11160







More information about the llvm-commits mailing list