Page MenuHomePhabricator

[GlobalISel][TableGen] Take first result for multi-output instructions
Needs ReviewPublic

Authored by ehjogab on Aug 26 2020, 5:52 AM.

Details

Summary

Previously, tblgen would reject patterns where one of its nested
instructions produced more than one result. These arise when the
instruction definition contains 'outs' as well as 'Defs'. This patch
fixes that by always taking the first result, which is how these
situations are handled in SelectionIDAG.

Diff Detail

Event Timeline

ehjogab created this revision.Aug 26 2020, 5:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2020, 5:52 AM
Herald added a subscriber: rovka. · View Herald Transcript
ehjogab requested review of this revision.Aug 26 2020, 5:52 AM

Needs test. I thought I handled this in ee3feef5aaaa3c385fbe08bdb2d48829ad440b56?

ehjogab updated this revision to Diff 294937.Sep 29 2020, 4:53 AM

Added testcase

ehjogab added a comment.EditedSep 29 2020, 4:54 AM

Needs test. I thought I handled this in ee3feef5aaaa3c385fbe08bdb2d48829ad440b56?

It seems that patch does not handle cases of nested instructions, which is the case in our target where this fix is needed.

I've added a testcase now, as requested.

Any comments? Should this patch be accepted or rejected?

Any comments? Should this patch be accepted or rejected?

I mostly see this as making the behavior more opaque by changing based on the specific register classes involved, so I'm not in favor. What do others think?

Any comments? Should this patch be accepted or rejected?

I mostly see this as making the behavior more opaque by changing based on the specific register classes involved, so I'm not in favor. What do others think?

I'd be happy to apply a better solution to this problem if you have one.

Added Paul-C-Anagnostopoulos since he's now one of the TableGen code owners.

I'm not yet familiar enough with instruction selection, so I'll leave the decision to others.