Page MenuHomePhabricator

[globalisel] Make the MatchAction hierarchy consistent with the matchers. NFC.
ClosedPublic

Authored by dsanders on Jan 31 2017, 2:23 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

dsanders created this revision.Jan 31 2017, 2:23 AM
kristof.beyls added inline comments.Jan 31 2017, 2:44 AM
utils/TableGen/GlobalISelEmitter.cpp
294 ↗(On Diff #86398)

For more consistency, maybe this class name should end in "Action", i.e. "MutateOpcodeAction"?
All of the matcher sub-class names end in "Matcher".

dsanders updated this revision to Diff 86403.Jan 31 2017, 3:05 AM

MutateOpcode -> MutateOpcodeAction

dsanders marked an inline comment as done.Jan 31 2017, 3:06 AM
dsanders added inline comments.
utils/TableGen/GlobalISelEmitter.cpp
294 ↗(On Diff #86398)

I agree. I've updated the patch

kristof.beyls added inline comments.Jan 31 2017, 5:25 AM
utils/TableGen/GlobalISelEmitter.cpp
330 ↗(On Diff #86403)

The corresponding function addPredicate(Args&&... args) has the same implementation, except for this return statement:

return *static_cast<Kind *>(Predicates.back().get());

Any reason why it needs to be different?

dsanders updated this revision to Diff 86427.Jan 31 2017, 7:10 AM
dsanders marked an inline comment as done.

Fixed a silly inconsistency between addPredicate() and addAction(). The return
value isn't used at the moment so I hadn't noticed I'd used MatchAction instead
of Kind.

dsanders marked an inline comment as done.Jan 31 2017, 8:12 AM
dsanders added inline comments.
utils/TableGen/GlobalISelEmitter.cpp
330 ↗(On Diff #86403)

Thanks for noticing. There's no reason behind it, it's just a silly mistake on my part. I've updated the patch

kristof.beyls added inline comments.Jan 31 2017, 8:43 AM
utils/TableGen/GlobalISelEmitter.cpp
330 ↗(On Diff #86403)

Thanks, the patch LGTM now.

This revision was automatically updated to reflect the committed changes.
dsanders marked an inline comment as done.