Date: Sun, 11 Nov 2012 22:53:48 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Dimitry Andric <dim@freebsd.org> Subject: Re: svn commit: r242835 - head/contrib/llvm/lib/Target/X86 Message-ID: <20121111214908.P938@besplex.bde.org> In-Reply-To: <509F2AA6.9050509@freebsd.org> References: <201211091856.qA9IuRxX035169@svn.freebsd.org> <509F2AA6.9050509@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 10 Nov 2012, Nathan Whitehorn wrote: > On 11/09/12 12:56, Dimitry Andric wrote: >> Log: >> Reduce LLVM's default stack alignment for i386 from 16 to 4 bytes, as >> the FreeBSD ABI requires. This is essentially a revert of upstream llvm >> commit r126226, and it will be reverted by upstream too. >> MFC after: 1 week >> >> Modified: >> head/contrib/llvm/lib/Target/X86/X86Subtarget.cpp >> >> Modified: head/contrib/llvm/lib/Target/X86/X86Subtarget.cpp >> ============================================================================== >> --- head/contrib/llvm/lib/Target/X86/X86Subtarget.cpp Fri Nov 9 18:23:38 >> 2012 (r242834) >> +++ head/contrib/llvm/lib/Target/X86/X86Subtarget.cpp Fri Nov 9 18:56:27 >> 2012 (r242835) >> @@ -416,12 +416,12 @@ X86Subtarget::X86Subtarget(const std::st >> assert((!In64BitMode || HasX86_64) && >> "64-bit code requested on a subtarget that doesn't support >> it!"); >> - // Stack alignment is 16 bytes on Darwin, FreeBSD, Linux and Solaris >> (both >> - // 32 and 64 bit) and for all 64-bit targets. >> + // Stack alignment is 16 bytes on Darwin, Linux and Solaris (both 32 and >> 64 >> + // bit) and for all 64-bit targets. >> if (StackAlignOverride) >> stackAlignment = StackAlignOverride; >> - else if (isTargetDarwin() || isTargetFreeBSD() || isTargetLinux() || >> - isTargetSolaris() || In64BitMode) >> + else if (isTargetDarwin() || isTargetLinux() || isTargetSolaris() || >> + In64BitMode) >> stackAlignment = 16; >> } >> > > I'd like to object to this. We have an identical ABI to Linux (and Solaris, > as far as I know). Splitting this by platform will only propagate the stack > alignment breakage further -- what we need is LLVM/our C library to handle > different alignments. Please fix this for real instead of balkanizing the ABI > support in LLVM and introducing different ABIs for LLVM and GCC to FreeBSD. I'm not sure if either of us knows exactly what this does, but I like this change. clang does stack alignment correctly, so it doesn't need the gcc pessimization of aligning in the caller. clang aligns in the callee if necessary (except if is lied to about the ABI, then it doesn't know what is necessaary). It is rarely necessary, since most variables are 32 bits or smaller. (Necessary includes when an alignment attribute says so and for optimal alignment of long longs and doubles. The i386 ABI doesn't require alignment for the latter, but optimization doesn't, and when the variables aren't in structs the ABI doesn't prevent them being aligned for optimization.) Always aligning in the caller wastes an average of not quite 8 bytes of stack and cache space per function call, plus sometimes an extra instruction or 2 to do the unnecessary alignment. gcc's alignment in callers doesn't even work, because gcc assumes that it is enough and never does it in callees even when it is necessary: auto int32_t foo[8] __aligned[32]; gcc fails to do the necessary alignment. clang does it. auto int32_t foo[8] __aligned[16]; Both gcc and (hopefully only without the above fix) clang fail to do the necessary alignment. The ABI doesn't require the stack to be 16-byte aligned, so alignment is necessary (unless -mpreferred-stack-boundary was used to explictly modify the ABI). gcc only aligns the stack in main(), only to the alignment specified by -mpreferred-stack-boundary, and laboriously passes it down to all functions. clang doesn't do any special alignment in main(), but assumes that crtso did it. clang also doesn't support -mpreferred-stack-boundary, so it is incompatible with nonstandard ABI's like the one given by always using -mpreferred-stack-boundary=32. auto int32_t foo[8] __aligned[8]; Both gcc and (hopefully only without the above fix) clang fail to do the necessary alignment. gcc doesn't even do it if you compile this with -mpreferred-stack-boundary=2, which should tell it not to assume 16-byte alignment. Re-quoting most of the above: > as far as I know). Splitting this by platform will only propagate the stack > alignment breakage further -- what we need is LLVM/our C library to handle > different alignments. Please fix this for real instead of balkanizing the ABI > support in LLVM and introducing different ABIs for LLVM and GCC to FreeBSD. Yes, we need clang/our libraries to handle different alignments and not assume that callers do more than the ABI requires and pessimize on behalf of the libraries. Outside of libraries, the problem is small provided -mpreferred-stack-boundary works, since you can compile everything with the same -mpreferred-stack-boundary. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121111214908.P938>