[llvm-commits] [llvm] r51134 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/2003-11-13-ConstExprCastCall.ll test/Transforms/InstCombine/fp-ret-bitcast.ll

[Dale Johannesen]

On May 15, 2008, at 10:26 AM, Duncan Sands wrote:

> Hi Dale,
>
>>> I don't get it.  Running the testcase under instcombine without your
>>> changes
>>> turns
>>>
>>> %tmp107 = call float bitcast (void (%struct.NSObject*, ...)*
>>> @objc_msgSend_fpret to float (%struct.NSObject*,
>>> %struct.objc_selector*)*)( %struct.NSObject* %tmp106,
>>> %struct.objc_selector* %tmp105 ) nounwind              ; <float>
>>> [#uses=0]
>>>
>>> into
>>>
>>>       call void (%struct.NSObject*, ...)*
>>> @objc_msgSend_fpret( %struct.NSObject* %tmp106,
>>> %struct.objc_selector* %tmp105 ) nounwind
>>>
>>> Since there is now no floating point value returned, how can there
>>> be a problem with
>>> removing it from the stack?  So I don't understand what this change
>>> is fixing.
>>
>> objc_msgSend_fpret does in fact return a floating point value on the
>> x87 stack (despite its quaint typing as "void").
>> Failure to pop this leads to stack overflow.
>
> are you saying that the declaration of objc_msgSend_fpret is wrong?
> That sounds like the real problem to me.  Having wrong declarations
> is fatal.  For example, suppose that the declaration was right: it
> does return void.  Then NOT performing the transform would be wrong,
> because otherwise you will pop a float that shouldn't be popped...
> Where is the declaration coming from?

That's the way the ObjC FE does things; it predates anyone working on  
llvm, and has tendrils all over gcc.  I don't like it either.  In  
theory I think you are right, but I doubt changing that is a practical  
approach.