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

[JF Bastien]

jfb added a comment.

I updated the patch to address all comment, except the two below from @rnk.

I'll update the description to accurately describe the current patch.


================
Comment at: lib/CodeGen/TargetRegisterInfo.cpp:315
@@ +314,3 @@
+                              F->hasFnAttribute(Attribute::StackAlignment));
+  if (requiresRealignment) {
+    if (canRealignStack(MF))
----------------
rnk wrote:
> Did the assertion not make it past the test suite? It looks like you added the dbgs() after this comment.
This is a non-fatal debug diagnostic, that'll only appear in a debug build with `-debug-only=target-reg-info`? It shouldn't hard-fail anything IIUC. Or am I misunderstanding?

================
Comment at: lib/Target/X86/X86RegisterInfo.cpp:473
@@ -472,3 +472,3 @@
   // it.
   if (MFI->hasVarSizedObjects())
     return MRI->canReserveReg(BasePtr);
----------------
rnk wrote:
> 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.
Would you mind if I do it in a separate patch? I'm assuming it'll require a test too :)


http://reviews.llvm.org/D11160