This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Separate MachineOperand implementation from MachineInstr
ClosedPublic

Authored by thegameg on Nov 24 2017, 3:33 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

thegameg created this revision.Nov 24 2017, 3:33 AM
thegameg updated this revision to Diff 124200.Nov 24 2017, 6:30 AM

Fix file header.

MatzeB accepted this revision.Nov 27 2017, 11:31 AM

LGTM, thanks. I'd commit the MachineOperand.h changes in a separate commit.

Feel free to do the doxygen comment cleanups in a separate future commit if you like.

lib/CodeGen/MachineOperand.cpp
10 ↗(On Diff #124200)

Use /// \file ... so doxygen can pick it up.

32–34 ↗(On Diff #124200)

This is unnecessary now as the whole file is about MachineOperand.

80 ↗(On Diff #124200)

As we are on it: Would be good to move the doxygen comments from the implementation to the header where possible.

113–115 ↗(On Diff #124200)

In cases where we have a doxygen comment in the implementation and the declaration we should just remove it from the implementation.

This revision is now accepted and ready to land.Nov 27 2017, 11:31 AM
This revision was automatically updated to reflect the committed changes.
thegameg marked 4 inline comments as done.
MatzeB added inline comments.Nov 28 2017, 11:04 AM
llvm/trunk/lib/CodeGen/MachineOperand.cpp
10–11

This should be:

/// \file Methods common to all machine operands.

(just look around some other files for examples).

thegameg marked an inline comment as done.Nov 28 2017, 11:17 AM
thegameg added inline comments.
llvm/trunk/lib/CodeGen/MachineOperand.cpp
10–11

Ok, fixed in r319206 (I actually did look around, but it looks like the version with a newline is more popular)