[cfe-commits] r126678 - in /cfe/trunk: lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/fpret.m test/CodeGenObjC/nonvararg-messaging-call.m test/CodeGenObjC/property-type-mismatch.m test/CodeGenObjC/property.m test/CodeGenObjC/variadic-sends.m test/CodeGenObjCXX/property-dot-reference.mm test/CodeGenObjCXX/property-object-conditional-exp.mm
jahanian
fjahanian at apple.com
Mon Feb 28 13:18:23 PST 2011
On Feb 28, 2011, at 1:12 PM, Chris Lattner wrote:
>
> On Feb 28, 2011, at 11:55 AM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Mon Feb 28 13:55:59 2011
>> New Revision: 126678
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=126678&view=rev
>> Log:
>> objc IRGen for Next runtime message API.
>> The prototype for objc_msgSend() is technically variadic -
>> `id objc_msgSend(id, SEL, ...)`.
>> But all method calls should use a prototype that matches the method,
>> not the prototype for objc_msgSend itself().
>> // rdar://9048030
>
> Hi Fariborz,
>
> If I understand this patch correctly, it is doing the wrong thing. We don't want to change the definition of objc_msgSend itself to be non-variadic, we want the compiler to cast the function pointer to the correct (non-variadic) function type for the receiver. With your patch, I see this:
>
> void foo(id x) {
> [x dowhatever: 4.0];
> }
>
> compile into:
>
> %call = call i8* bitcast (i8* (i8*, i8*)* @objc_msgSend to i8* (i8*, i8*, double)*)(i8* %tmp, i8* %tmp1, double 4.000000e+00)
>
> which is technically incorrect, because objc_msgSend *actually is* variadic. Without your patch I see:
>
> %call = call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to i8* (i8*, i8*, double)*)(i8* %tmp, i8* %tmp1, double 4.000000e+00)
>
> which is already what we want: objc_msgSend is variadic, but it is cast to the correct nonvariadic type for the call. I recently fixed an optimizer bug that caused the optimizer to fold this away, introducing the extraneous "xor al, al".
>
> Anyway, long story short, I don't think this patch is needed.
OK I will revert it. I assume that you patch is in TOT. I will try the test case with -O after undoing the patch.
- fariborz
>
> -Chris
>
>
>
>
>
More information about the cfe-commits
mailing list