This is an archive of the discontinued LLVM Phabricator instance.

[mips] MIPS64R6 compact branch support
ClosedPublic

Authored by sdardis on May 11 2016, 6:15 AM.

Details

Summary

MIPS64R6 compact branch support. As the MIPS LLVM backend uses distinct
MachineInstrs for certain 32 and 64 bit instructions (e.g. BEQ & BEQ64) that
map to the same operation, extend compact branch support for the
corresponding 64bit branches.

Diff Detail

Event Timeline

sdardis updated this revision to Diff 56894.May 11 2016, 6:15 AM
sdardis retitled this revision from to [mips] MIPS64R6 compact branch support.
sdardis updated this object.
sdardis set the repository for this revision to rL LLVM.
sdardis added a subscriber: llvm-commits.
dsanders requested changes to this revision.May 16 2016, 8:17 AM
dsanders added a reviewer: dsanders.
dsanders added inline comments.
lib/Target/Mips/Mips64r6InstrInfo.td
138–139

We shouldn't be using isCodeGenOnly to avoid decoder conflicts and should use DecoderNamespace instead. One thing to be careful of is that some encodings like BGEC_ENC already has a DecoderNamespace to handle the unusual shapes (such as the rs < rt conditions) in the encoding table (see DecodeDisambiguates and DecodeDisambiguatedBy).

This revision now requires changes to proceed.May 16 2016, 8:17 AM
sdardis updated this revision to Diff 57640.May 18 2016, 9:17 AM
sdardis edited edge metadata.
sdardis removed rL LLVM as the repository for this revision.

Used decoder name space instead of isCodeGenOnly.

sdardis updated this revision to Diff 60660.Jun 14 2016, 2:23 AM
sdardis edited edge metadata.

Refresh to near ToT, implement encoding restrictions.

dsanders requested changes to this revision.Jun 22 2016, 9:04 AM
dsanders edited edge metadata.

The code change looks correct to me but there's some bugs in the test cases.

test/CodeGen/Mips/compactbranches/beqc-bnec-register-constraint.ll
17

It doesn't make much functional difference in this case but can you add a ':' so that we match the label instead of a nearby directive. Likewise for the other CHECK-LABEL's

test/CodeGen/Mips/compactbranches/compact-branches-64.ll
6

The various checking directives can match in the wrong function at the moment. Could you add some CHECK-LABEL directives to prevent this?

11

There aren't any RUN lines that enable the CHECK prefix (it's only the default when --check-prefix isn't given).

15–16

There aren't any RUN directives that specify the 'STATIC' prefix.

40

This one is missing a ':'

63

The whitespace between the 'CHECK' and ':' disables this check

187

There are no RUN directives that enable STATIC64

test/CodeGen/Mips/compactbranches/no-beqzc-bnezc.ll
10–57

This block isn't doing what you expect. It's checking that the two *-NOT's don't occur between the first two 'f' characters in the output. One of them will probably match the 'f' in the '.file' directive and it's quite likely that both occur before the 'f:' label on the first function

71

f1 -> f4

89

f2 -> f5

This revision now requires changes to proceed.Jun 22 2016, 9:04 AM
sdardis updated this revision to Diff 61664.Jun 23 2016, 4:57 AM
sdardis edited edge metadata.

Fixed tests.

Just realised I missed fixing a CHECK, will upload new diff in a bit.

sdardis updated this revision to Diff 61667.Jun 23 2016, 5:37 AM
sdardis edited edge metadata.

Fixed tests again.

dsanders added inline comments.Jun 27 2016, 3:41 AM
test/CodeGen/Mips/compactbranches/compact-branches-64.ll
13

The CHECK prefix is still disabled so we're not actually performing these checks

sdardis updated this revision to Diff 64288.Jul 18 2016, 2:43 AM

Fix labels in tests.

dsanders accepted this revision.Jul 25 2016, 8:44 AM
dsanders edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jul 25 2016, 8:44 AM
This revision was automatically updated to reflect the committed changes.