This is an archive of the discontinued LLVM Phabricator instance.

[TableGen][RISCV] Relax a restriction in generating patterns for commutable SDNodes.
ClosedPublic

Authored by craig.topper on Jan 22 2022, 10:03 AM.

Details

Summary

Previously, all children would be checked to see if any were an
explicit Register. If anywhere no commutable patterns would be
generated. This patch loosens the restriction to only check the
children that are being commuted.

Digging back through history, this code predates the existence of
commutable intrinsics and commutable SDNodes with more than 2
operands. At that time the loop would count the number of children that
weren't registers and if that was equal to 2 it would allow commuting.
I don't think this loop was re-considered when commutable
intrinsics were added or when we allowed SDNodes with more than 2
operands.

This important for RISCV were our isel patterns have a V0 mask
operand after the commutable operands on some RISCVISD opcodes.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 22 2022, 10:03 AM
craig.topper requested review of this revision.Jan 22 2022, 10:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2022, 10:03 AM

Ah great, I was going to do this today! My local patch wasn't as ambitious and didn't merge the intrinsic/non-intrinsic paths though. Makes sense to me. I didn't get around to doing any diffs on the generated tables - does anything but RISCV change?

llvm/utils/TableGen/CodeGenDAGPatterns.cpp
4648–4649

Several lines here could potentially be simplified if we had a unsigned offset = isCommIntrinsic ? 1 : 0 or somesuch. Then we could assert(N->getNumChilden() >= 2 + offset), i = offset, e = 2 + offset?

Simplify code as suggested.

arsenm accepted this revision.Feb 1 2022, 5:08 PM
This revision is now accepted and ready to land.Feb 1 2022, 5:08 PM
This revision was landed with ongoing or failed builds.Feb 1 2022, 9:22 PM
This revision was automatically updated to reflect the committed changes.