Page MenuHomePhabricator

[AMDGPU] Shrink MAD/FMA to MADAK/MADMK/FMAAK/FMAMK on GFX10
ClosedPublic

Authored by foad on May 13 2022, 10:21 AM.

Details

Summary

On GFX10 VOP3 instructions can have a literal operand, so the conversion
from VOP3 MAD/FMA to VOP2 MADAK/MADMK/FMAAK/FMAMK will not happen in
SIFoldOperands. The only benefit of the VOP2 form is code size, so do it
in SIShrinkInstructions instead.

Diff Detail

Event Timeline

foad created this revision.May 13 2022, 10:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 10:21 AM
foad requested review of this revision.May 13 2022, 10:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2022, 10:21 AM
rampitec added inline comments.May 13 2022, 12:07 PM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
348

A leftover?

355

Add some text here.

369

Same here...

376

Same here...

399

You bail if hasAnyModifiersSet().

rampitec added inline comments.May 13 2022, 12:08 PM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
399

Oh, I see. Ignore this comment.

foad updated this revision to Diff 429654.May 16 2022, 2:11 AM

Address review comments.

foad marked 6 inline comments as done.May 16 2022, 2:14 AM
foad added inline comments.
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
348

No, it was intentional, but I agree it looked a bit strange. I have changed it.

foad updated this revision to Diff 429658.May 16 2022, 2:31 AM
foad marked an inline comment as done.

Prefer not to swap operands when possible, for efficiency and to make
some before/after code comparisons easier.

foad updated this revision to Diff 429664.May 16 2022, 2:57 AM

Copy flags from MI to NewMI.

arsenm added inline comments.May 16 2022, 6:06 AM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
394

Can use .add on the usual BuildMI chain

foad updated this revision to Diff 429698.May 16 2022, 6:16 AM

Use more MachineInstrBuilder methods.

foad marked an inline comment as done.May 16 2022, 6:17 AM
arsenm accepted this revision.May 16 2022, 6:18 AM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
809

Are you going to do this in a separate change?

This revision is now accepted and ready to land.May 16 2022, 6:18 AM
foad added inline comments.May 16 2022, 6:22 AM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
809

Sure, can do, but there are no in-tree tests for it, and no use for it in the graphics shaders I've been looking at either.

This revision was landed with ongoing or failed builds.May 16 2022, 7:26 AM
This revision was automatically updated to reflect the committed changes.
foad added inline comments.May 17 2022, 9:15 AM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
809