Page MenuHomePhabricator

[AMDGPU][GlobalISel] Transform (fadd (fmul x, y), z) -> (fma x, y, z)
Needs ReviewPublic

Authored by matejam on Dec 15 2020, 8:21 AM.

Details

Summary

Instead of add (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 simple case that has only one add and one mul instruction
and transforms them into some of the fma instructions depending on the
architecture.

Diff Detail

Event Timeline

matejam created this revision.Dec 15 2020, 8:21 AM
matejam requested review of this revision.Dec 15 2020, 8:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 8:21 AM
arsenm added inline comments.Dec 15 2020, 3:43 PM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
3145–3146

Using bf16 here instead of regular half is wrong. We also have a utility function to get the FP type from the LLT

3159

Checking the denormal mode here isn't right. The denormal mode only impacts whether this should be used for AMDGPU and isn't a generic property

3185–3187

return compare directly

3196

The matching logic here implies this should be able to use fma also

matejam updated this revision to Diff 323356.Fri, Feb 12, 9:24 AM
matejam retitled this revision from [AMDGPU][GlobalISel] Add combiner for generating G_FMAD to [AMDGPU][GlobalISel] Add combiners for v_mad/v_mac/v_fma.

Instead of add 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 simple case that has only one add and one mul instruction
and transforms them into some of the fma instructions depending on the
architecture.

matejam retitled this revision from [AMDGPU][GlobalISel] Add combiners for v_mad/v_mac/v_fma to [AMDGPU][GlobalISel] Transform (fadd (fmul x, y), z) -> (fma x, y, z).Fri, Feb 12, 9:28 AM
matejam edited the summary of this revision. (Show Details)
arsenm added inline comments.Fri, Feb 12, 9:42 AM
llvm/include/llvm/CodeGen/TargetLowering.h
796

This will break for vectors

1116

This will break for vectors. Also, this should not be built on top of the DAG legalizer rules. This should be something in LegalizerInfo

2744

Broken for vectors

foad added inline comments.Fri, Feb 12, 10:49 AM
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
112

isLegal needs its own comment.

333

Comment should mention fmad.

334

Capitalise them like "FAdd" and "FMul" and "FMA" and "FMad". There are akready examples of this in GlobalISel.

550

"FMul"

llvm/include/llvm/CodeGen/TargetLowering.h
795

Function needs a comment.

798

Do you need EVT() here? I thought MVT would implicitly convert to EVT.

1114

Function needs a comment.

2693

Function needs a comment.

2734

Function needs a comment.

2739

Function needs a comment.

llvm/include/llvm/Target/GlobalISel/Combine.td
560

"combine_fadd_fmul_to_fmad_or_fma"?

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
3575

Function needs a comment.