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

[Eric Christopher]

On Wed, Jul 29, 2015 at 4:53 PM Vasileios Kalintiris <
Vasileios.Kalintiris at imgtec.com> wrote:

> vkalintiris added a comment.
>
> Hi Eric,
>
> Thanks for your comments. I replaced -mips-fast-isel with sed and forgot
> to do the same for -fast-isel. It should be fine now.
>
> 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
>
> ----------------
> echristo wrote:
> > Ditto.
> I believe that this one is required because of the -O2 level. Otherwise, I
> get the following assert error:
>
> ```
> Assertion `(!EnableFastISelAbort || TM.Options.EnableFastISel) &&
> "-fast-isel-abort > 0 requires -fast-isel"
> ```
>
> Would you like me to change the level and whatever CHECK-lines are needed
> in this review? There are several things that we have to clean-up in
> FastISel and it's my intention to do that after the upcoming release.
>
>
Do you actually mean to be using fast isel with optimization? It's fine if
you are, but it's not a typical test route and you should probably comment
it.



> ================
> 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
>
> ----------------
> echristo wrote:
> > The typical thing is to just use llc -march here, but I gather
> optimization makes the testcase go away?
> That's right. However, we need the function's attributes too. This fine
> balance of options forces the OptLevelChanger to change the function's
> opt-level from -O2 to -O0. With -debug-only=isel, I get the following
> output:
>
>
Weird on the optimization, I'm dubious here. Can you take a look at it?

(and yes, that's what optnone means)

Also, I meant that you could do the cleanup separately of the attribute
move. If you need to change the attributes as part of the patch it's
something to raise during review.

-eric


> ```
> ...
> Changing optimization level for Function main
>         Before: -O2 ; After: -O0
> ....
> ```
>
> I had to try by trial and error in order to get it right, after reading
> about this behaviour from lib/CodeGen/SelectionDag/SelectionDagISel.cpp and
> the comments at test/CodeGen/X86/dag-optnone.ll
>

This
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150730/89e9f172/attachment.html>