[PATCH] D24824: [mips][FastISel] Instantiate the MipsFastISel class only for targets that support FastISel.

[Simon Dardis via llvm-commits]

sdardis requested changes to this revision.
sdardis added a comment.
This revision now requires changes to proceed.

Comments inlined.


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:464-465
@@ +463,4 @@
+
+  // FastISel is supported only for [MIPS32,MIPS32R6) ISAs.
+  bool UseFastISel = TM.Options.EnableFastISel && Subtarget.hasMips32();
+
----------------
I find this comment strange given the block beneath. I think this should be:

   // FastISel is supported only for [MIPS32,MIPS32R5] ISAs.
   bool UseFastISel = TM.Options.EnableFastISel && Subtarget.hasMips32() && !Subtarget.hasMips32r6());


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:467-469
@@ +466,5 @@
+
+  // Disable FastISel if we use MIPS32R6 or any microMIPS mode.
+  if (Subtarget.hasMips32r6() || Subtarget.inMicroMipsMode())
+    UseFastISel = false;
+
----------------
With my comment above, I think this should check for inMicroMips(), with the comment reflecting that we don't support compressed instruction sets.

Aside, I've barely looked but is mips16e rejected at a higher level for FastISel?


https://reviews.llvm.org/D24824