[llvm-commits] [PATCH] Proper treatment of unhandled CCValAssign cases in X86FastISel.cpp

[Chad Rosier]

Applied in clang revision 160055.

 Chad

On Jul 11, 2012, at 10:02 AM, Chad Rosier wrote:

> 
> On Jul 11, 2012, at 8:20 AM, Kaylor, Andrew wrote:
> 
>> Sure.  I will try to whittle my reproducer down to a manageable test case.  I probably won’t have time to get to it until next week, but I will try to put something together.
>>  
>> I can tell you that my reproducer crashes if I simply break from the switch on the CCValAlign::Indirect case.  That is, if I make this change:
>>  
>> Index: X86FastISel.cpp
>> ===================================================================
>> --- X86FastISel.cpp  (revision 160015)
>> +++ X86FastISel.cpp  (working copy)
>> @@ -1693,7 +1693,7 @@
>>      // Promote the value if needed.
>>      switch (VA.getLocInfo()) {
>> -    default: llvm_unreachable("Unknown loc info!");
>> +    default: return false;
>> +    case CCValAssign::Indirect: break;
>>      case CCValAssign::Full: break;
>>      case CCValAssign::SExt: {
>>        assert(VA.getLocVT().isInteger() && !VA.getLocVT().isVector() &&
>>  
>> Is that what you intended?
> 
> Yes.  Given that it crashes I'll go ahead and apply the original patch.  Please make sure to send the test case, so I can take a look a creating a fix that doesn't bail from fast-isel.
> 
>  Chad
> 
>> -Andy
>>  
>> From: Chad Rosier [mailto:mcrosier at apple.com] 
>> Sent: Tuesday, July 10, 2012 5:45 PM
>> To: Kaylor, Andrew
>> Cc: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] [PATCH] Proper treatment of unhandled CCValAssign cases in X86FastISel.cpp
>>  
>> Andrew,
>> Would you mind providing a test case?  Certainly, returning false will work, but I'm wondering if we need to bail on fast-isel.  
>>  
>> The switch is promoting values when necessary, but I don't think that's necessary for CCValAssign::Indirect.  In that case I believe you can just break from the switch.  However, I don't know if the rest of fast-isel is able to handle this case.
>>  
>>  Chad
>>  
>> On Jul 10, 2012, at 5:19 PM, Kaylor, Andrew wrote:
>> 
>> 
>> Hello all,
>>  
>> I came across a case today where the X86 fast instruction selector was throwing an llvm_unreachable error in response to a valid value for an argument location (CCValAssign::Indirect in the case I ran into).  I don’t know if it would be reasonable to handle this case in the X86FastISel code, but in the meantime it seems the code should simply return ‘false’ when it encounters an unimplemented location type so that the SelectionDAG can handle it.
>>  
>> The attached patch does that.
>>  
>> -Andy
>>  
>> <X86FastISel.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>  
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120711/4802dd0c/attachment.html>