This is an archive of the discontinued LLVM Phabricator instance.

Sparc back-end: Addition of missing registers and associated instructions.
ClosedPublic

Authored by lero_chris on Feb 11 2016, 2:50 AM.

Details

Reviewers
jyknight
venkatra
Summary

The patch adds missing registers and instructions to complete all the registers supported by the Sparc v8 manual. These are all co-processor registers, with the exception of the floating-point deferred-trap queue register.

Diff Detail

Event Timeline

lero_chris updated this revision to Diff 47616.Feb 11 2016, 2:50 AM
lero_chris retitled this revision from to Sparc back-end: Addition of missing registers..
lero_chris updated this object.
lero_chris added reviewers: venkatra, jyknight.
lero_chris added a subscriber: llvm-commits.
jyknight edited edge metadata.Feb 11 2016, 2:29 PM

I'd like for the change that adds the instructions that use these registers to be merged with this change; this is not actually useful standing by itself.

lib/Target/Sparc/SparcRegisterInfo.td
214

You can't just make up Dwarf reg numbers, these have to match with other tools like gdb and gcc. And, there aren't any defined for coproc registers, so just leave the DwarfRegNum out.

385

This should probably be marked isAllocatable = 0.

lero_chris updated this revision to Diff 47783.Feb 12 2016, 2:50 AM
lero_chris retitled this revision from Sparc back-end: Addition of missing registers. to Sparc back-end: Addition of missing registers and associated instructions..
lero_chris updated this object.
lero_chris edited edge metadata.

Made modifications to code per comments from jyknight.

Added instruction implementations and tests verifying the new instructions are encoded correctly. Note: These instructions are expected to be used exclusively in inline assembler and no automatic lowering for the instructions is defined.

Overall looks good, just a couple minor tweaks.

There should be some MC/Disassembler tests added as well. (One might hope that the same tests would be shared for assembly and disassembly, but that's not actually the case, unfortunately.)

lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
153

"CoPro" seems an unusual shortening of Coprocessor. "Coproc" seems better.

test/MC/Sparc/sparc-copro.s
3 ↗(On Diff #47783)

Seems an odd selection of tests: %c4/%i1 %c4/%i7 %c19/%i1 all test the same instruction definition, so I don't really see the point of including all of them. I'd include, instead a reg+reg and a reg+imm for each of these new instructions.

58 ↗(On Diff #47783)

Last 3 tests misplaced? %fq doesn't go with coprocessor tests, and %y already had tests.

Made amendments following feedback: (1) Added disassembler tests. (2) Moved new FP instruction to correct test file (3) Changed "CoPro" to "Coproc" in both code and filenames. (4) Small white-space formatting changes.

jyknight accepted this revision.Feb 23 2016, 6:54 AM
jyknight edited edge metadata.
This revision is now accepted and ready to land.Feb 23 2016, 6:54 AM
lero_chris closed this revision.Mar 7 2016, 5:52 AM

Checked-in as revision 262135