This is an archive of the discontinued LLVM Phabricator instance.

[MIPatternMatch]: Add matchers for binary instructions
ClosedPublic

Authored by Petar.Avramovic on Apr 1 2021, 8:05 AM.

Details

Summary

Add matchers that support commutative and non-commutative binary opcodes.

Diff Detail

Event Timeline

Petar.Avramovic requested review of this revision.Apr 1 2021, 8:05 AM
Petar.Avramovic created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2021, 8:05 AM

Testcase?

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
258

Commutative?

268

Maybe a bit simpler to change this to the negative case?

Also may want to check that there is only one def.

279
322
Petar.Avramovic edited the summary of this revision. (Show Details)

Simplify tests that fail because of non-binary instruction (match register instead of m_ICst) since they fail when number of def/use operands is checked. Update commit message.

foad added a comment.Apr 26 2021, 4:10 AM

I'm not sure the "AnyBinaryOp" versions are really needed for anything -- see https://reviews.llvm.org/D90050#inline-955365.

As for BinaryOpWithOpcode_match, couldn't you just change the existing BinaryOp_match to take the opcode as a regular argument instead of a template argument? (Are you worried about making it slower? Hopefully with proper inlining there would be no codegen difference at all.)

Petar.Avramovic edited the summary of this revision. (Show Details)

Removed matchers that don't check opcode.
My guess is that this was originally done as a equivalent of IR/PatternMatch.h.
Thus i also added AnyBinaryOp_match alongside with the version that checks opcode which i originally wanted(named BinaryOpc_match here).
I thought that AnyBinaryOp_match would be useful when we know opcode already so we avoid checking it again, but turns out that at the moment this is not that useful.
I would prefer to leave existing code with opcodes as template arguments like in IR/PatternMatch.h and add new matcher that has opcode as an regular argument instead. m_Add ect. are meant to have templated opcode and changing it to regular opcode argument almost guaranteed results in a same thing but brings no improvement.

foad accepted this revision.Apr 26 2021, 6:18 AM

I would prefer to leave existing code with opcodes as template arguments like in IR/PatternMatch.h and add new matcher that has opcode as an regular argument instead.

Fair enough, if no-one else objects to that approach.

This revision is now accepted and ready to land.Apr 26 2021, 6:18 AM
This revision was landed with ongoing or failed builds.Apr 27 2021, 2:38 AM
This revision was automatically updated to reflect the committed changes.