[PATCH] D11249: Fix PR#24142

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.

================
Comment at: lib/CodeGen/TargetInstrInfo.cpp:664
@@ -663,2 +663,3 @@
   int SPAdj = MI->getOperand(0).getImm();
+  SPAdj = TFI->alignSPAdjust(SPAdj);
 
----------------
rnk wrote:
> I don't think this is the right place to fix this. getSPAdjust is also used on X86 to return SP adjustment values for PUSH, POP, CALL, and others. These typically produce values that are not a multiple of stack alignment (4 / 8 vs 16).
I think this actually is the right place for it.
This looks like a long-standing bug from before I factored this code out of PEI and into TFI.

From my commit message on the refactoring, emphasis is new:
"PEI tries to keep track of how much starting or ending a call sequence adjusts the stack pointer by, so that it can resolve frame-index references. Currently, it takes a very simplistic view of how SP adjustments are done - both FrameStartOpcode and FrameDestroyOpcode adjust it exactly by the amount written in its first argument.

This view is in fact incorrect for some targets (*e.g. due to stack re-alignment*, or because it may want to adjust the stack pointer in multiple steps). However, that doesn't cause breakage, because most targets (the only in-tree exception appears to be 32-bit ARM) rely on being able to simplify the call frame pseudo-instructions earlier, so this code is never hit. "

The newly-created (in a later commit) X86 version of getSPAdjust actually does take alignment into account, but, unfortunately, I never went back and fixed the ARM version - I didn't know it's broken in practice.


http://reviews.llvm.org/D11249