[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]

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
> 
> 
> 
> 
>