This is an archive of the discontinued LLVM Phabricator instance.

TableGen/GlobalISel: Allow inst matcher to check multiple opcodes
ClosedPublic

Authored by arsenm on Aug 4 2020, 5:41 AM.

Details

Summary

This is to initially handleg immAllOnesV, which should match
G_BUILD_VECTOR or G_BUILD_VECTOR_TRUNC. In the future, it could be
used for other patterns cases that map to multiple G_* instructions,
such as G_ADD and G_PTR_ADD.

Diff Detail

Event Timeline

arsenm created this revision.Aug 4 2020, 5:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2020, 5:41 AM
arsenm requested review of this revision.Aug 4 2020, 5:41 AM
foad added a subscriber: foad.Aug 6 2020, 2:17 AM
foad added inline comments.
llvm/utils/TableGen/GlobalISelEmitter.cpp
1674–1678

Typo "one several".

arsenm updated this revision to Diff 283672.Aug 6 2020, 11:07 AM

Fix comment

arsenm marked an inline comment as done.Aug 6 2020, 11:07 AM

Could we have more than 2 possible choices for opcodes? Would it make sense to make this take an arbitrary number of alternatives?

Maybe something like

/// - InsnID - Instruction ID
/// - NumChoices - Number of possible opcodes
/// - Opcode1, Opcode2, Opcode3, ... OpcodeNumChoices - Possible opcodes, ordered by preference
GIM_CheckOpcodeWithAlternatives

I can't think of anything off the top of my head, but it could be useful I suppose.

Could we have more than 2 possible choices for opcodes? Would it make sense to make this take an arbitrary number of alternatives?

Maybe something like

/// - InsnID - Instruction ID
/// - NumChoices - Number of possible opcodes
/// - Opcode1, Opcode2, Opcode3, ... OpcodeNumChoices - Possible opcodes, ordered by preference
GIM_CheckOpcodeWithAlternatives

I can't think of anything off the top of my head, but it could be useful I suppose.

I considered doing it this way, but I couldn't come up with a case where we would need more than 2. I'd rather use the simpler thing unless there's a concrete need

Could we have more than 2 possible choices for opcodes? Would it make sense to make this take an arbitrary number of alternatives?

Maybe something like

/// - InsnID - Instruction ID
/// - NumChoices - Number of possible opcodes
/// - Opcode1, Opcode2, Opcode3, ... OpcodeNumChoices - Possible opcodes, ordered by preference
GIM_CheckOpcodeWithAlternatives

I can't think of anything off the top of my head, but it could be useful I suppose.

I considered doing it this way, but I couldn't come up with a case where we would need more than 2. I'd rather use the simpler thing unless there's a concrete need

Ok, that seems reasonable to me.

Do you think you can change the name from GIM_CheckOpcode2 to something more descriptive?

paquette accepted this revision.Aug 6 2020, 4:08 PM

LGTM

This revision is now accepted and ready to land.Aug 6 2020, 4:08 PM
arsenm updated this revision to Diff 286050.Aug 17 2020, 9:29 AM

Fix breaking SwitchOpcode optimization