Page MenuHomePhabricator

[AMDGPU] Fixed v_swap_b32 match
ClosedPublic

Authored by rampitec on Fri, Oct 16, 3:28 PM.

Details

Summary
  1. Fixed liveness issue with implicit kills.
  2. Fixed potential problem with an indirect mov.

Fixes: SWDEV-256848

Diff Detail

Event Timeline

rampitec created this revision.Fri, Oct 16, 3:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Oct 16, 3:28 PM
rampitec requested review of this revision.Fri, Oct 16, 3:28 PM

Couldn't this try to preserve the implicit operands?

Couldn't this try to preserve the implicit operands?

It can be implicit def or implicit kill. In both cases verifier will complain if it is dropped. Then the problem is there can be other instructions in between of the combined three and correct licenses may be needed in that range. That is probably possible to preserve implicit operands on the move at the insertion point and carry it over to the swap, but I am not so sure about the other two. Insert there an IMPLICIT_DEF or KILL?

rampitec updated this revision to Diff 299467.Tue, Oct 20, 2:12 PM

Preserve implicit operands on the mov at insertion point.

rampitec updated this revision to Diff 299492.Tue, Oct 20, 3:24 PM

Preserve implicit operands.

arsenm added inline comments.Wed, Oct 21, 9:08 AM
llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp
442–447

Rather than special casing exec, this should check if the number of operands matches the number in the MCInstrDesc

rampitec updated this revision to Diff 299721.Wed, Oct 21, 9:24 AM
rampitec marked an inline comment as done.

Do not specaial case implicit exec.

arsenm accepted this revision.Wed, Oct 21, 9:57 AM
This revision is now accepted and ready to land.Wed, Oct 21, 9:57 AM
This revision was automatically updated to reflect the committed changes.