[PATCH] Implement shift ops for Mips fast-isel

[Eric Christopher]

Replies inline.

-eric


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1297
@@ +1296,3 @@
+  switch (DVT.SimpleTy) {
+  default:
+    // Trunc i64 to i32 is handled by the target-independent fast-isel.
----------------
rkotler wrote:
> echristo wrote:
> > I'd add this assert:
> > 
> > assert((DVT.SimpleTy != MVT::i32 && DVT.SimpleTy != MVT::i64) && "i64 and i32 should have been handled by target-independent fast-isel!");
> If we want to add that then why not just a simple LLVM_Unreachable ??
Because SimpleTy are more than the two you have there and you'd like to make sure that all of the possible types that come in are handled rather than say that you've handled all of them?

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1448
@@ +1447,3 @@
+    default:
+      llvm_unreachable("Unexpected instruction.");
+    case Instruction::Shl:
----------------
rkotler wrote:
> echristo wrote:
> > Is this an actual error or what?
> I think it should be impossible to get that because we are only calling this function upon seeing one of those instructions. I think I added that case to avoid compiler warnings when compiling this.
> 
> In fastSelectionInstruction
> 
>   case Instruction::Shl:
>   case Instruction::LShr:
>   case Instruction::AShr:
>     return selectShift(I);
> 
> 
Makes sense. Go ahead and make your comment more descriptive of what instructions you expect to see.

http://reviews.llvm.org/D6726

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/