Page MenuHomePhabricator

[mips] Expansion of BEQL and BNEL with immediate operands
ClosedPublic

Authored by sdardis on Feb 9 2016, 1:54 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

obucina updated this revision to Diff 47360.Feb 9 2016, 1:54 PM
obucina retitled this revision from to [mips] Expansion of BEQL and BNEL with immediate operands.
obucina updated this object.
obucina added reviewers: dsanders, zoran.jovanovic.
obucina added a subscriber: llvm-commits.
vkalintiris edited edge metadata.Mar 1 2016, 3:43 AM

Overall this looks good to me. However, we'll have to add a few more tests before committing this. You can take a look at r239905 in order to get an idea of what we need to change:

test/MC/Mips/branch-pseudos-bad.s
test/MC/Mips/branch-pseudos.s
test/MC/Mips/set-nomacro.s

Can You please be more precise what exactly You are expecting to be changed?

branch-pseudo.s is a set of tests for branch instructions with register operands. I guess You think we need tests for global labels?

branch-pseudo-bad.s also covers branch instructions with register operands. I guess You want to test for AT reg availability here?

set-nomacro.s covers branch instructions, but not branch likely instructions? Do You want to add these two instructions here? For the sake of completeness, what about other branch likely instructions?

Just be more precise, please...

obucina updated this revision to Diff 50056.Mar 8 2016, 10:57 AM
obucina edited edge metadata.

Aditional tests added.

vkalintiris accepted this revision.Mar 21 2016, 9:33 AM
vkalintiris edited edge metadata.

LGTM, with two small comments inline.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2581 ↗(On Diff #50056)

This line is too big, can you split it?

test/MC/Mips/macro-bcc-imm.s
6–37 ↗(On Diff #50056)

We don't need the duplicate tests using the bar label.

This revision is now accepted and ready to land.Mar 21 2016, 9:33 AM
obucina updated this revision to Diff 51295.Mar 22 2016, 9:51 AM
obucina edited edge metadata.
obucina marked 2 inline comments as done.

According to feedback - split long line, redundant tests removed.

seanbruno requested changes to this revision.Sep 26 2016, 6:48 AM
seanbruno added a reviewer: seanbruno.
seanbruno added a subscriber: seanbruno.

This patch has aged a bit and doesn't apply cleanly to trunk any longer.

Can this be refreshed to a more recent trunk?

This revision now requires changes to proceed.Sep 26 2016, 6:50 AM

Hrm ... I think this one fell off the radar a bit.

sdardis commandeered this revision.Jan 30 2017, 3:14 AM
sdardis edited reviewers, added: obucina; removed: sdardis.
sdardis edited edge metadata.

Taking over this patch.

sdardis updated this revision to Diff 86265.Jan 30 2017, 3:16 AM
sdardis edited edge metadata.
sdardis edited the summary of this revision. (Show Details)

Updated to trunk, added more tests.

seanbruno accepted this revision.Jan 30 2017, 2:59 PM
This revision is now accepted and ready to land.Jan 30 2017, 2:59 PM
This revision was automatically updated to reflect the committed changes.