This is an archive of the discontinued LLVM Phabricator instance.

Add Simple return instruction to Mips fast-isel
ClosedPublic

Authored by rkotler on Apr 18 2014, 2:51 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

Did you mean to update D3404 rather than create a new patch? It looks like it's the same change with the test case added and EnableMipsFastISel defaulted to true.

LGTM with a couple whitespace nits. There's also a query as to whether the change to enable -mips-fast-isel by default was intentional.

This is an update D3404 that was accidentally submitted as a new patch. I'll close D3430

lib/Target/Mips/MipsISelLowering.cpp
56 ↗(On Diff #8655)

As discussed, I'm in favour of it being enabled by default but I ought to ask if it was intentional.

test/CodeGen/Mips/Fast-ISel/nullvoid.ll
1–2 ↗(On Diff #8655)

There's trailing whitespace on the second RUN line.
Could you also add a couple spaces before the '<'? It gives another clue that it's a continued line.

10–11 ↗(On Diff #8655)

Blank lines at EOF

I had changed init to true because management instructed me to do so.

But I've been arguing for it to be false.

But it seems everyone else wants it to be true.

I will enable it to be true but submit that as a separate one line patch.

dsanders accepted this revision.Apr 25 2014, 1:52 AM
dsanders edited edge metadata.

It seems I forgot to mark this patch accepted. Done

This revision is now accepted and ready to land.Apr 25 2014, 1:52 AM
rkotler updated this revision to Diff 8865.Apr 26 2014, 12:27 PM
rkotler edited edge metadata.
  • fix test case so that it only works if all fast-isel patterns are matched. also add one line omitted from select ret that is needed in all other ports for fast-isel
rkotler updated this revision to Diff 8866.Apr 26 2014, 12:29 PM

Clean up git error

rkotler updated this revision to Diff 8867.Apr 26 2014, 12:31 PM
  • get rid of uneeded file

-O0 was need for the test case in order to enable -fast-isel. It would also have been possible to just add -fast-isel.
-fast-isel-abort requires that -fast-isel is already enabled.

The new version LGTM

rkotler updated this revision to Diff 8928.Apr 29 2014, 8:55 AM

Make -fast-isel off for now by default for Mips

New version LGTM

rkotler updated this revision to Diff 8929.Apr 29 2014, 9:04 AM

need mips-fast-isel to enable test - fix test case

rkotler closed this revision.May 19 2014, 7:23 AM
rkotler updated this revision to Diff 9545.

Closed by commit rL207565 (authored by @rkotler).