[PATCH] D11610: [mips][FastISel] Remove hidden mips-fast-isel option.

[Vasileios Kalintiris]

Hi Daniel,

I tried to take a quick look into the issue but I can't figure out exactly what's going on. I will commit this so that we can merge it to the release branch. I completely agree with you that in principle -fast-isel=false shouldn't produce any different output with/without this patch. However, I'll look it more thoroughly over the weekend by tracing the execution of llc with and without FastISel enabled.

Thanks,
V. Kalintiris
________________________________________
From: Daniel Sanders
Sent: 30 July 2015 12:07
To: Vasileios Kalintiris; Daniel Sanders
Cc: echristo at gmail.com; qcolombet at apple.com; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] D11610: [mips][FastISel] Remove hidden mips-fast-isel option.

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

It will LGTM with the attribute issue resolved. It seems excessive for this small detail to block merging to the release branch though so I don't mind if you commit and resolve that bit in a follow up.


================
Comment at: test/CodeGen/Mips/Fast-ISel/sel1.ll:2
@@ -1,3 +1,3 @@
 ; RUN: llc < %s -march=mipsel -mcpu=mips32r2 -O2 -relocation-model=pic \
-; RUN:          -fast-isel -mips-fast-isel -fast-isel-abort=1 | FileCheck %s
+; RUN:          -fast-isel -fast-isel-abort=1 | FileCheck %s

----------------
I agree that this one is necessary since Fast ISel isn't the default for -O2

================
Comment at: test/CodeGen/Mips/emergency-spill-slot-near-fp.ll:2
@@ -1,3 +1,3 @@
 ; Check that register scavenging spill slot is close to $fp.
-; RUN: llc -march=mipsel -O0 < %s | FileCheck %s
+; RUN: llc -march=mipsel -O0 -fast-isel=false < %s | FileCheck %s

----------------
> However, we need the function's attributes too.

I don't entirely follow why we need the attributes. FastISel off-by-default and on-by-default but explicitly disabled should have the same effect.
Also, the 'Changing optimization level' message you mentioned looks like it's just applying the effect of the -O0 command line option.

There's some dubious looking code in OptLevelChanger's constructor that enables FastISel without consulting the command-line option like LLVMTargetMachine.cpp's addPassesToGenerateCode does. Are we sure that -fast-isel=false works?


http://reviews.llvm.org/D11610