Page MenuHomePhabricator

MIRParser: Allow register class specification on operand
ClosedPublic

Authored by MatzeB on Jul 14 2016, 8:18 PM.

Details

Summary

You can now define the register class of a virtual register on the
operand itself avoiding the need to use a "registers:" block.

Example: "%0:gr64 = COPY %rax"

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 64085.Jul 14 2016, 8:18 PM
MatzeB retitled this revision from to MIRParser: Allow register class specification on operand.
MatzeB updated this object.
MatzeB added reviewers: qcolombet, arphaman.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.
qcolombet added inline comments.Jul 22 2016, 12:54 PM
lib/CodeGen/MIRParser/MIParser.cpp
138 ↗(On Diff #64085)

parserRegisterClassOrBank?

851 ↗(On Diff #64085)

We should look for register banks as well.

1010 ↗(On Diff #64085)

I believe we should have the same syntax for register bank.

test/CodeGen/MIR/X86/register-operand-class.mir
12 ↗(On Diff #64085)

We shouldn’t need to emit that part anymore, right?
Probably a follow-up patch, though.

MatzeB updated this revision to Diff 81808.Dec 16 2016, 3:39 PM

Updated to handle regbanks.

MatzeB marked 3 inline comments as done.Dec 16 2016, 3:40 PM
MatzeB added inline comments.
test/CodeGen/MIR/X86/register-operand-class.mir
12 ↗(On Diff #64085)

yep, that is for a followup and needs some policy decisions like what to do with virtual registers that aren't used anywhere.

MatzeB marked an inline comment as done.Jan 10 2017, 4:15 PM

ping

qcolombet accepted this revision.Jan 10 2017, 5:27 PM
qcolombet edited edge metadata.

LGTM

test/CodeGen/MIR/X86/register-operand-class.mir
19 ↗(On Diff #81808)

Just a thought, shouldn't we make the class/bank on the definition mandatory?

This revision is now accepted and ready to land.Jan 10 2017, 5:27 PM

Thanks for the review.

test/CodeGen/MIR/X86/register-operand-class.mir
19 ↗(On Diff #81808)

Yeah, enforcing this for every def (and function live-in declaration) would be a good thing. First however we need a patch that lets the MIRPrinter use the new syntax here.

This revision was automatically updated to reflect the committed changes.