[llvm-commits] [llvm][PATCH][Review Requested] Fix failure to realign stack on x86

[Gurd, Preston]

Thank you for your swift response, Jakob.

I am not the originator of this patch, but I have been tasked with getting it (or a better version) committed to the LLVM trunk.

The original problem came up when trying to run JITed code using 16 byte aligned memory accesses on x86 Android, where, for whatever reason, the stack pointer was not aligned mod 16 on entry.

The "StackAlign" variable in the code seems to indicate the desired stack alignment, rather than the actual stack alignment at runtime. If that is true, then the expression

        MFI->getMaxAlignment() > StackAlign

does not actually indicate whether or not the actual stack pointer needs realignment. The proposed patch (changing > to >=) seems to just force realignment to be done.

The test case  is run with -realign-stack set to either 0 or 1. If -realign-stack=1 means that the function entry code should cause the stack pointer to be aligned on entry, then it does so only with the patch.

If you can suggest a cleaner way to address this problem, please let me know.

Preston



From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk]
Sent: Wednesday, January 18, 2012 1:30 PM
To: Gurd, Preston
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm][PATCH][Review Requested] Fix failure to realign stack on x86


On Jan 18, 2012, at 9:55 AM, Gurd, Preston wrote:


The attached one line patch corrects a problem whereby llvm was failing to generate an ANDL instruction to align the stack pointer on function entry when the required alignment was mod 16 and when stack realignment was requested.

--- lib/Target/X86/X86RegisterInfo.cpp        (revision 148345)
+++ lib/Target/X86/X86RegisterInfo.cpp     (working copy)
@@ -352,7 +352,7 @@
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   const Function *F = MF.getFunction();
   unsigned StackAlign = TM.getFrameLowering()->getStackAlignment();
-  bool requiresRealignment = ((MFI->getMaxAlignment() > StackAlign) ||
+  bool requiresRealignment = ((MFI->getMaxAlignment() >= StackAlign) ||
                                F->hasFnAttr(Attribute::StackAlignment));


I don't understand this change. Why realign the stack when it is already sufficiently aligned?

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120118/91aac6af/attachment.html>