[llvm-commits] [llvm] r160055 - /llvm/trunk/lib/Target/X86/X86FastISel.cpp

[Chad Rosier]

On Jul 11, 2012, at 11:26 AM, Eric Christopher wrote:

> 
> On Jul 11, 2012, at 11:19 AM, Chad Rosier <mcrosier at apple.com> wrote:
> 
>> 
>> On Jul 11, 2012, at 11:05 AM, Eric Christopher wrote:
>> 
>>> I think I'd almost rather leave the unreachable as the default and add anything that's missing (with possibly a TODO).
>>> 
>>> Your thoughts?
>> 
>> I actually wrote that exact patch, but then applied the original.  Reason being this seems to be a very rare case (base on the fact we just got a complaint now), so I wasn't sure if it was worth fixing.  I'm fine with making the suggested change, however.
> 
> I'm ambivalent, I _think_ I'd prefer the former rather than the one that was applied just because it makes it more obvious when something new is added that we should do something. That said an ICE isn't very friendly to end users. :)

I agree, the llvm_unreachable shouldn't be there if we can fall back to Selection DAG isel.  I'll just add all the cases to the switch with FIXMEs and remove the default.

> -eric