[cfe-commits] r116696 - in /cfe/trunk: lib/CodeGen/TargetInfo.cpp test/CodeGen/x86_32-arguments-darwin.c test/CodeGen/x86_32-arguments-linux.c

[Daniel Dunbar]

On Mon, Oct 18, 2010 at 4:42 PM, Bill Wendling <isanbard at gmail.com> wrote:
> On Oct 18, 2010, at 4:16 PM, Daniel Dunbar wrote:
>
>> Hi Bill,
>>
>> On Sun, Oct 17, 2010 at 8:41 PM, Bill Wendling <isanbard at gmail.com> wrote:
>>> Author: void
>>> Date: Sun Oct 17 22:41:31 2010
>>> New Revision: 116696
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=116696&view=rev
>>> Log:
>>> Reapply r116684 with fixes. The test cases needed to be updated.
>>
>> I like the general approach, but I don't think any of the changes to
>> propagate neededMMX are, err, needed. Actually, I think they are wrong
>> because we won't stop using registers for varargs at the right point
>> any more unless I am misreading things.
>>
> I was wondering that too. I think it's useful in this part:
>
>>> @@ -1787,7 +1817,7 @@
>>>   FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
>>>
>>>   // Keep track of the number of assigned registers.
>>> -  unsigned freeIntRegs = 6, freeSSERegs = 8;
>>> +  unsigned freeIntRegs = 6, freeSSERegs = 8, freeMMXRegs = 8;
>>>
>>>   // If the return value is indirect, then the hidden argument is consuming one
>>>   // integer register.
>>> @@ -1798,16 +1828,18 @@
>>>   // get assigned (in left-to-right order) for passing as follows...
>>>   for (CGFunctionInfo::arg_iterator it = FI.arg_begin(), ie = FI.arg_end();
>>>        it != ie; ++it) {
>>> -    unsigned neededInt, neededSSE;
>>> -    it->info = classifyArgumentType(it->type, neededInt, neededSSE);
>>> +    unsigned neededInt, neededSSE, neededMMX;
>>> +    it->info = classifyArgumentType(it->type, neededInt, neededSSE, neededMMX);
>>>
>>>     // AMD64-ABI 3.2.3p3: If there are no registers available for any
>>>     // eightbyte of an argument, the whole argument is passed on the
>>>     // stack. If registers have already been assigned for some
>>>     // eightbytes of such an argument, the assignments get reverted.
>>> -    if (freeIntRegs >= neededInt && freeSSERegs >= neededSSE) {
>>> +    if (freeIntRegs >= neededInt && freeSSERegs >= neededSSE &&
>>> +        freeMMXRegs >= neededMMX) {

The problem is here, the SSE and MMX counts need to be the same so we
stop using registers when we exceed 8 combined, not 8 of either.

 - Daniel

>>>       freeIntRegs -= neededInt;
>>>       freeSSERegs -= neededSSE;
>>> +      freeMMXRegs -= neededMMX;
>>>     } else {
>>>       it->info = getIndirectResult(it->type);
>>>     }
>
> But it was this part that I was unsure about, so I made it and neededSSE equivalent:
>
>>> @@ -1876,10 +1908,13 @@
>>>   //   i8* overflow_arg_area;
>>>   //   i8* reg_save_area;
>>>   // };
>>> -  unsigned neededInt, neededSSE;
>>> +  unsigned neededInt, neededSSE, neededMMX;
>>>
>>>   Ty = CGF.getContext().getCanonicalType(Ty);
>>> -  ABIArgInfo AI = classifyArgumentType(Ty, neededInt, neededSSE);
>>> +  ABIArgInfo AI = classifyArgumentType(Ty, neededInt, neededSSE, neededMMX);
>>> +
>>> +  // Lump the MMX in with SSE.
>>> +  neededSSE += neededMMX;
>>>
>>>   // AMD64-ABI 3.5.7p5: Step 1. Determine whether type may be passed
>>>   // in the registers. If not go to step 7.
>
> Basically, the variable then comes down to use in X86_64ABIInfo::computeInfo...
>
> -bw
>
>