[PATCH] D134961: [AMDGPU][MC][GFX11] Correct v_fmac_.*_e64_dpp

Stanislav Mekhanoshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 3 09:50:15 PDT 2022


rampitec added a comment.

In D134961#3830760 <https://reviews.llvm.org/D134961#3830760>, @Joe_Nash wrote:

> In D134961#3830721 <https://reviews.llvm.org/D134961#3830721>, @dp wrote:
>
>>> If I understand the current state: e32 dpp is not creatable but is assemblable, e64 dpp is  creatable but not assemblable.
>>
>> Correct. I'm not sure e64 dpp is creatable but at least it has $old operand.
>>
>>> Perhaps a way to fix _e64 is in the asmParser in cvtVOP3DPP ~L8900? I think it's treating the operand as src2 when it is actually the old operand.
>>
>> cvtVOP3DPP incorrectly handles $old because it is not tied to $dst.
>>
>> It seems possible to change cvtVOP3DPP to handle v_fmac opcodes correctly. But if we go that way, $src2 will be tied to $dst, but $old will be present but not tied. Is this acceptable?
>
> I believe it is actually that way currently in main. And it seems to work in GCNDPPCombine at least. Though I have not strongly tested it, maybe it's wasting a register? @rampitec
>
> Tablegen dump yields : 
> def V_FMAC_F32_e64_dpp
>
>   ...
>   dag InOperandList = (ins anonymous_9483:$old ...
>   ...
>   string Constraints = "$vdst = $src2";
>   string DisableEncoding = "$src2";

Do we really need both? src2 and old are basically the same thing here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134961/new/

https://reviews.llvm.org/D134961



More information about the llvm-commits mailing list