This is an archive of the discontinued LLVM Phabricator instance.

[TableGen][GlobalISel] Add Generic MatchTableExecutor Emitter
ClosedPublic

Authored by Pierre-vh on Jun 26 2023, 4:53 AM.

Details

Summary

Move all of the reusable logic out of GlobalISelEmitter.cpp into a GlobalISelMatchTableExecutorEmitter class so the future combiner backend can use it as well.

Depends on D153755

Diff Detail

Event Timeline

Pierre-vh created this revision.Jun 26 2023, 4:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2023, 4:53 AM
Herald added a subscriber: mgrang. · View Herald Transcript
Pierre-vh requested review of this revision.Jun 26 2023, 4:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2023, 4:53 AM
Pierre-vh retitled this revision from [TableGen] Add Generic MatchTableExecutor Emitter to [TableGen][GlobalISel] Add Generic MatchTableExecutor Emitter.Jun 26 2023, 5:02 AM
Pierre-vh updated this revision to Diff 534887.Jun 27 2023, 2:50 AM

Allow custom AdditionalDecls in emitMIPredicateFns

arsenm added inline comments.Jun 28 2023, 5:28 AM
llvm/utils/TableGen/GlobalISelEmitter.cpp
302

Make final?

314

Probably should make this return a StringRef and make clients not allocate temporary strings on every debug print

2271

MF should probably be available as a class member, set in setupMF. Long term I don't think instructions should point to their parents

2272

setupMF already does this but for some reason is just repeated by every target, should just move that to the base class

2302

any way to avoid duplicating all the strings?

llvm/utils/TableGen/GlobalISelMatchTableExecutorEmitter.cpp
63

C++17 tuple syntax?

Pierre-vh updated this revision to Diff 537017.Jul 4 2023, 2:59 AM
Pierre-vh marked 6 inline comments as done.

Comments

llvm/utils/TableGen/GlobalISelEmitter.cpp
2272

I tried putting MRI into the base class, but then it needs to be a pointer, which means all combines that use MRI (and there are lot of them, I checked) need to take that into account and use */->, I think it doesn't really give us anything to move MRI to the base
For the combiners I can just remove it because all combiners need MRI already

aemerson accepted this revision.Jul 6 2023, 1:39 PM

Seems reasonable to me.

This revision is now accepted and ready to land.Jul 6 2023, 1:39 PM