[PATCH] [PATCH] Fix for http://llvm.org/bugs/show_bug.cgi?id=19905

Sanjoy Das sanjoy at playingwithpointers.com
Sat Jun 7 23:35:49 PDT 2014


>>! In D4052#4, @vadimcn wrote:
> Hi Sanjay,
> Please take a look at my preliminary patch to support Win64 SEH I've put here: http://reviews.llvm.org/D4053 (specifically stuff in X86FrameLowering.cpp).   Not directly related to 19905, however, it touches the same code, and could benefit from the same refactoring.

Yes, I think you could do away with MaxSpillSlotOffset once this
change (or something like this) is in, and just use getObjectOffset.
This is assuming I correctly understand what MaxSpillSlotOffset is
used for. :)

> 
> Firstly, I think that spill slots will have to go into fixed frame slots, because in order to describe them in terms of Win64 SEH directives, the spilled XMM registers need to be addressable from the frame pointer.   Right now they end up on the wrong side of the "stack realignment gap" in cases when stack is realigned.

I don't know anything about the Win64 ABI, so I won't comment on this.
However, I think the above issue is orthogonal to this change.

> 
> Also, as I was working on this code, a couple of questions popped in my mind:
> 1. Why does PrologueEpilogueInserter pre-assign spill slots to registers before calling spillCalleeSavedRegisters?   Wouldn't it be more logical to let spillCalleeSavedRegisters do that (only if it is going to return true, of course).

I think PEI pre-assigns frame indices, not actual spill slots.  Also
spillCalleeSavedRegisters is a TargetFrameLowering function, and the
logic to assign frame indices is target agnostic, so maybe that's the
reason for putting it in PEI.  A point to note is that this issue
doesn't have any direct relation to eager / early assignment of frame
indices.

> 2. Wouldn't it make sense to move the actual emission of PUSHes and MOVs into emitPrologue?   I think this would make CFI/SEH info generation much more straightforward.  spillCalleeSavedRegisters would still have to assign the spill slots, of course.   The only downside I see, is that this code wouldn't be able to use TII.storeRegToStackSlot(), because by that point frame slots have already been reified.

At this time I don't have enough insight to comment on the pros and
cons of such a change; especially whether such a change simplifies x86
code at the cost of making other backends more complex.

http://reviews.llvm.org/D4052






More information about the llvm-commits mailing list