[llvm-commits] [llvm] r173325 - /llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp

[Reed Kotler]

Thanks guys! Nice work that Clang picked up on this. We have been using 
gcc still for our builds. Time to switch!

Was in a hurry to push the change and get onto the next task and did not 
do a good self code review. I think that in reality those places can 
never be reached but I need to review this today. Right now I'm just
doing pic O32 but there is N32 which has some different calling 
semantics. This mips16 hard float exercise already uncovered one bug in
in our mips32 compiler calling convention processing and after today, I 
think I see some other small issues. I need to compare what we are doing 
with gcc.

This code is to make up for the lack of floating point instructions 
while in Mips16 mode when it calls an external routine which can in 
reality turn out to be either mips32 or mips16.

I need to double check on the meaning of Args, RetTy in the cases below.
I think that if the return type is a non complex struct that the struct
is already being treated as a hidden first argument but I'm not sure.
If the first argument is not float or double and the return type is not
float, double, single or double complex, then no helper function is needed.

If RetTy can be a struct but also non _Complex, then the situation that 
is really uncovered is

struct_foo xyz(float [, .....])
struct_foo xyz(double [,.....])
struct_foo xyz(float, float, [,.....])
struct_foo xyz(float, double, [......])
struct_foo xyz(double, float, [......])
struct_foo xyz(double, double, [......])

In these cases, no helper function is needed if the address of the 
struct will be passed as a hidden first parameter. If struct is being
passed back in integer registers, then we have the same case as void 
xyz(float) or int xyz(float).

My test case is minimal and I had not gotten to testing all these cases 
yet. I have a lot more cases I want to add but I'm not sure if I thought 
about adding this one or not. You can never over-test a compiler!

Because I always call this with needHelper=false, that's why it was 
working despite this.


On 01/23/2013 10:45 PM, NAKAMURA Takumi wrote:
> David, yeah, it makes sense.
>
> I really wanted to fill the paths, then. :)
>
> ...Takumi
>
> 2013/1/24 David Blaikie <dblaikie-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org>:
>> I believe the correct (stylistically) change here is to change the 'if' to
>> 'assert', rather than adding else-unreachable
>>
>> On Jan 23, 2013 10:11 PM, "NAKAMURA Takumi" <geek4civic-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org> wrote:
>>>
>>> Author: chapuni
>>> Date: Thu Jan 24 00:08:06 2013
>>> New Revision: 173325
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=173325&view=rev
>>> Log:
>>> MipsISelLowering.cpp: Fill unreachable paths to fix warnings.
>>> [-Wsometimes-uninitialized]
>>>
>>> FIXME: Could they, unreachable(s), be removed?
>>> FIXME: I could prefer the coding standards...
>>>
>>> Modified:
>>>      llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp
>>>
>>> Modified: llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp?rev=173325&r1=173324&r2=173325&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp (original)
>>> +++ llvm/trunk/lib/Target/Mips/MipsISelLowering.cpp Thu Jan 24 00:08:06
>>> 2013
>>> @@ -2917,6 +2917,12 @@
>>>                  (RetTy->getContainedType(1)->isDoubleTy())) {
>>>           result = dcMips16Helper[stubNum];
>>>         }
>>> +      else {
>>> +        llvm_unreachable("Uncovered condition");
>>> +      }
>>> +    }
>>> +    else {
>>> +      llvm_unreachable("Uncovered condition");
>>>       }
>>>     }
>>>     else {
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits-Tmj1lob9twqVc3sceRu5cw at public.gmane.org
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits