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

Simon Dardis via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 8 11:40:46 PDT 2016


sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LTGM, with one comment nit, inlined.



================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:464-465
+
+  // We support only the standard encoding [MIPS32,MIPS32R6) ISAs.
+  bool UseFastISel = TM.Options.EnableFastISel && Subtarget.hasMips32() &&
+                     !Subtarget.hasMips32r6() && !Subtarget.inMips16Mode() &&
----------------
ehostunreach wrote:
> That was intentional as a coding style, ie. set UseFastISel and use the checks that follow to unset it. I merged the ISA-checks together.
My request here is that the comment on what ISAs we support doesn't mention MIPS32R6. So that comment should be:

   // We support only the standard encoding [MIPS32,MIPS32R5] ISAs.

Otherwise the comments don't quite match the code. IMHO.




https://reviews.llvm.org/D24824





More information about the llvm-commits mailing list