This is an archive of the discontinued LLVM Phabricator instance.

MIRParser: Rewrite register info initialization; mostly NFC
ClosedPublic

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

Details

Summary

This changes MachineRegisterInfo to be initializes after parsing all
instructions. This is in preparation for upcoming commits that allow the
register class specification on the operand or deduce them from the
MCInstrDesc.

This commit removes the unused feature of having nonsequential register
numbers. This was confusing anyway as the vreg numbers would be
different after parsing when you had "holes" in your numbering.

This patch also introduces the concept of an incomplete virtual
register. An incomplete virtual register may be used during .mir parsing
to construct MachineOperands without knowing the exact register class
(or register bank) yet.

NFC except for some error messages.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 64084.Jul 14 2016, 8:17 PM
MatzeB retitled this revision from to MIRParser: Rewrite register info initialization; mostly NFC.
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:10 PM
lib/CodeGen/MIRParser/MIParser.cpp
52 ↗(On Diff #64084)

Could we maintain a map of "virtual ID” to actual vreg number instead of creating a bunch of useless ones?
Although, that’s probably cheap, I don’t like the idea of a bunch of undef registers in the middle.

Also, that should reduce the number of incomplete virtual registers, since we would create registers only at use/def points and we will know the register class there for the most part.

lib/CodeGen/MIRParser/MIParser.h
39 ↗(On Diff #64084)

explicitly

lib/CodeGen/MachineFunctionAnalysis.cpp
58 ↗(On Diff #64084)

Unrelated to the patch (+ I pushed a fix for that yesterday ;))

test/CodeGen/MIR/X86/undefined-virtual-register.mir
22 ↗(On Diff #64084)

For that specific case, I would prefer a different message. Like %1 never defined.
For cases where the register is properly defined, but we can’t deduce the type with the use/def (like here with copy), I believe we shouldn’t call that a type. We should use register class/bank.

What I am saying is that we should reserve the word “type” for instruction types like sN, unsized, etc.

test/CodeGen/MIR/X86/unused-virtual-register.mir
14 ↗(On Diff #64084)

I would get rid of that test. I basically don’t care if we don’t use dense numbering.

MatzeB marked 5 inline comments as done.Jul 26 2016, 5:52 PM

Thanks for the review! New patch is coming.

lib/CodeGen/MIRParser/MIParser.cpp
52 ↗(On Diff #64084)

I went without a map to give the user control over the exact vreg numbers he will end up with.
Thinking about this some more it is probably not an important feature to have and having a map does indeed simplify writing down some tests.

lib/CodeGen/MachineFunctionAnalysis.cpp
58 ↗(On Diff #64084)

that went in by accident.

test/CodeGen/MIR/X86/unused-virtual-register.mir
14 ↗(On Diff #64084)

Obsolete and removed with the new patch.

MatzeB updated this revision to Diff 65639.Jul 26 2016, 5:53 PM
MatzeB marked 3 inline comments as done.

Updated patch that maps vreg numbers in the .mir file to newly created vregs in the MachineFunction (as opposed to taking them literally as the previous version).

qcolombet accepted this revision.Oct 6 2016, 11:01 AM
qcolombet edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Oct 6 2016, 11:01 AM
This revision was automatically updated to reflect the committed changes.