[llvm-commits] [PATCH] Add support for functions with VLAs and dynamic stack realignment on x86

[Chad Rosier]

Jakob,
I believe I've addressed all your comments.  I also ran the nightly test suite with and without this patch and verified that it does not change codegen for any benchmark.

  Chad

-------------- next part --------------
A non-text attachment was scrubbed...
Name: x86-VLAs-dynamic-realignment.patch
Type: application/octet-stream
Size: 14392 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120604/0441b6af/attachment.obj>
-------------- next part --------------


On Jun 1, 2012, at 10:46 AM, Jakob Stoklund Olesen wrote:

> 
> On Jun 1, 2012, at 10:13 AM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>>> Index: lib/Target/X86/X86RegisterInfo.cpp
>>> 
>>> +    if (CC == CallingConv::GHC) {
>>> +      report_fatal_error(
>>> 
>>> Could you check for the actual feature here, instead of testing specifically for GHC? Check out TRI::getCallPreservedMask() and the static MO::clobbersPhysReg().
>> 
>> I'm not sure I follow.  Could I call TRI::getCalleeSavedRegs() and then check to see if this is a non-empty set?  Then I could select one of these registers or would it be better to define the register statically (currently, X86::RBX)?
> 
> The base pointer must be a callee-saved register in the current calling convention, that is why GHC won't work. You can check that like this:
> 
>  if (MachineOperand::clobbersPhysReg(getCallPreservedMask(CC), getBaseRegister()))
>    report_fatal_error;
> 
> I guess it would be even better to pick a different base pointer register, but I don't think that is necessary in the first patch.
> 
>>> +    Reserved.set(X86::RBX);
>>> +    Reserved.set(X86::EBX);
>>> +    Reserved.set(X86::BX);
>>> +    Reserved.set(X86::BL);
>>> +    Reserved.set(X86::BH);
>>> 
>>> What about the configurable getBaseRegister()? Look at MCSubRegIterator.
>> 
>> I'm using the MCSubRegIterator now, but I'm not sure this addresses your comment.  Is that what you were suggesting?
> 
> What if getBaseRegister() returns something other than RBX? You shouldn't hardcode RBX, that's all I meant.
> 
> /jakob
>