This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][GlobalISel] Transform (fsub (fpext (fneg (fmul x, y))), z) -> (fneg (fma (fpext x), (fpext y), z))
ClosedPublic

Authored by matejam on Mar 5 2021, 7:51 AM.

Details

Summary

Instead of sub and mul instructions, use v_mad, v_mac or v_fma if fma
instructions are faster and are legal for the given architecture.
Combiner for a case that has one multiplication, which gets negated and
extended, and from that the third argument is subtracted. This combiner
will transform that into two extends and one negation within the fma/fmad
instruction.

Diff Detail

Event Timeline

matejam created this revision.Mar 5 2021, 7:51 AM
matejam requested review of this revision.Mar 5 2021, 7:51 AM
matejam updated this revision to Diff 329340.Mar 9 2021, 7:19 AM

Put back the accidentally deleted combiner from the list of combiners (load_or_combine).

arsenm requested changes to this revision.Mar 31 2021, 4:23 PM
arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4591–4592

First, fadd only has as single type index. Second, I don't think there's much point in checking it's legality. It doesn't imply "LegalOperations"

4598

We have a separate check for before the legalizer. FMA also only has one type index

4637

Should probably be using mi_matches for all of these instead of your own getVRegDefs and nested ifs

This revision now requires changes to proceed.Mar 31 2021, 4:23 PM
matejam updated this revision to Diff 343377.May 6 2021, 5:50 AM

Minor changes in CombinerHelper.cpp and in the tests.

arsenm added inline comments.
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4835

Should have mi_match support for fneg/fpext and use that

matejam updated this revision to Diff 346782.May 20 2021, 10:24 AM

Use mi_match for comparing instructions instead of comparing the opcodes.

matejam updated this revision to Diff 346784.May 20 2021, 10:25 AM

Use mi_match for comparing instructions instead of comparing the opcodes.

matejam updated this revision to Diff 375877.Sep 29 2021, 7:10 AM

Use m_MInstr instead of m_Reg in matching patterns (mi_match).
Formatting and refactoring.

matejam updated this revision to Diff 376210.Sep 30 2021, 7:10 AM

Use applyBuildFn instead of writing my own apply.

matejam updated this revision to Diff 376524.Oct 1 2021, 7:24 AM

Changes in tests.

matejam updated this revision to Diff 390336.Nov 29 2021, 6:22 AM

Added missing comments and an FMA combiner group in table gen.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 29 2021, 7:28 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.