x86-64 doesn't save nonvolatile XMM6-XMM15 registers

[llvmbot]

Bugzilla Link	2801
Resolution	FIXED
Resolved on	Mar 05, 2009 14:36
Version	unspecified
OS	Windows XP
Blocks	llvm/llvm-bugzilla-archive#3739
Reporter	LLVM Bugzilla Contributor
CC	@asl

Extended Description

The x86-64 ABI specifies that XMM6 to XMM15 are non-volatile, and should be preserved by the callee as needed. It appears that LLVM currently doesn't do this, causing unpredictable behavior with floating-point calculations.

[llvmbot]

I've got things working by updating X86RegisterInfo::getCalleeSavedRegs and getCalleeSavedRegClasses and having spillCalleeSavedRegisters and restoreCalleeSavedRegisters return false (which causes the prolog/epilog inserter to use generic stores and loads to preserve the registers on the stack).

I'll attempt to write a proper patch now that only changes the behavior for Win64 (other ABI's specify all XMM registers to be scratch).

[llvmbot]

Which x86-64 ABI are you talking about? AMD64 ABI shows this:

%xmm2–%xmm7 used to pass floating point arguments No
%xmm8–%xmm15 temporary registers No

The last column is "no" which means these registers are not preserved across function calls.

[llvmbot]

Ah, is this Win64?

[llvmbot]

The issue is specific to Windows; Darwin and Linux are correct.

[asl]

The issue is specific to Windows; Darwin and Linux are correct.
Win64 support is not yet complete. It contains several flaws, which prevents from
their full use. I'm going to dig into this problem, when I will have time. However, this will require some huge CC lowering revamp.

[llvmbot]

Preserve XMM registers on Win64
Here's a simple patch that works for me.

I didn't succeed yet in extending spillCalleeSavedRegisters and restoreCalleeSavedRegisters to also properly take care of the XMM registers. I was able to generate movaps instructions using storeRegToStackSlot and loadRegToStackSlot but the stack is not 16-byte aligned so I got an exception. The workaround in the patch is to just let saveCalleeSavedRegisters use the generic approach with mov instructions instead of push/pop. There might be a tiny performance drop for Win64 but having working code is more important than speed for now.

[asl]

Created an attachment (id=2024) [details]
Preserve XMM registers on Win64

Here's a simple patch that works for me.

I didn't succeed yet in extending spillCalleeSavedRegisters and
restoreCalleeSavedRegisters to also properly take care of the XMM registers. I
was able to generate movaps instructions using storeRegToStackSlot and
loadRegToStackSlot but the stack is not 16-byte aligned so I got an exception.
The workaround in the patch is to just let saveCalleeSavedRegisters use the
generic approach with mov instructions instead of push/pop. There might be a
tiny performance drop for Win64 but having working code is more important than
speed for now.
I definitely missed "high" part of xmm registers for win64. However, I don't understand, why you say, that stack is not 16-byte aligned. It is so on win64 and if vector have proper alignment, it will be saved to aligned spill slot. Could you please provide a testcase?

[llvmbot]

I definitely missed "high" part of xmm registers for win64. However, I don't
understand, why you say, that stack is not 16-byte aligned. It is so on win64
and if vector have proper alignment, it will be saved to aligned spill slot.
Could you please provide a testcase?

Microsoft changed it's own Win64 specs without notice; now the entire XMM register has to be preserved. Anyway, you're right, the stack itself is 16-byte aligned. But the offsets where the XMM registers are stored/loaded are not. I'll attach the incorrect patch I used in a second. I'm not sure if I can find a compact testcase but I'll see what I can do.

[llvmbot]

Incorrect patch storing XMM registers at unaligned addresses
Here's the incorrect patch I was talking about. Instead of relying on PEI::saveCalleeSavedRegisters to use generic stores and loads I tried to let X86InstrInfo::spillCalleeSavedRegisters and X86InstrInfo::restoreCalleeSavedRegisters take care of storing and loading XMM registers as well, using storeRegToStackSlot and loadRegFromStackSlot. Unfortunately this can cause attempts to store or load them at unaligned offsets (e.g. [esp+8]).

The other patch works for me though, it just might be a tiny bit slower because it doesn't use push/pop. So if it looks good to you could you commit it for me so we at least have something that works in the meantime? Thanks.

[asl]

Nicolas, I applied the callee-saved register part of your patch. See r56568.

I believe, that alignment of stack variables is separate issue. Please check, if there are any PRs opened already on this problem, and if not - fill new PR with testcase.

[llvmbot]

Nicolas, I applied the callee-saved register part of your patch. See r56568.
I believe, that alignment of stack variables is separate issue. Please check,
if there are any PRs opened already on this problem, and if not - fill new PR
with testcase.

Thanks for the commit. Unfortunately, without the rest of my patch spillCalleeSavedRegisters now generates push instructions for saving the XMM registers (and restoreCalleeSavedRegisters generates pop instructions). So for example what should have been something like "movaps [esp+20h], xmm12" becomes "push r12".

I couldn't find any PR's about stack variable alignment, but I'm not sure if that's the source of the problem in the first place. I just don't know how to properly build up the stack frame with LLVM. Anyway, I'll provide a testcase so you can at least see what's currently generated...

[llvmbot]

mentioned in issue llvm/llvm-bugzilla-archive#3739