Page MenuHomePhabricator

[AMDGPU] Fixed dpp combine of VOP1
ClosedPublic

Authored by rampitec on Wed, Oct 9, 2:08 PM.

Details

Summary

If original instruction did not have source modifiers they were
not added to the new DPP instruction as well, even if needed.

Diff Detail

Event Timeline

rampitec created this revision.Wed, Oct 9, 2:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Oct 9, 2:08 PM
arsenm added inline comments.Wed, Oct 9, 2:22 PM
llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
221–222

This case isn't tested

llvm/test/CodeGen/AMDGPU/dpp_combine.mir
530

Add a comment explaining what this tests

rampitec marked an inline comment as done.Wed, Oct 9, 2:23 PM
rampitec added inline comments.
llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
221–222

I do not think such instructions currently exists. VOP2 are tested by the original test.

arsenm added inline comments.Wed, Oct 9, 2:33 PM
llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
221–222

Won't this happen for any VOP2 form of an FP instruction? V_MIN_F32_e32 has no modifiers, but V_MIN_F32_dpp does

rampitec marked 4 inline comments as done.Wed, Oct 9, 2:45 PM
rampitec added inline comments.
llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
221–222

Good point, thanks!

rampitec updated this revision to Diff 224175.Wed, Oct 9, 2:45 PM
rampitec marked an inline comment as done.

Updated test.

arsenm accepted this revision.Wed, Oct 9, 2:58 PM

LGTM

This revision is now accepted and ready to land.Wed, Oct 9, 2:58 PM
This revision was automatically updated to reflect the committed changes.