[PATCH] D133012: [AMDGPU] Add subtarget feature for MAD_U64/I64 bug on GFX11

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added a comment.

In D133012#3934097 <https://reviews.llvm.org/D133012#3934097>, @foad wrote:

> I have often wanted a standard way of mapping two different pseudos to the same real instruction.

I guess that is useful, but the behavior here is not always available. We are basically having multiple maps overloaded onto the same keys.
Here we are using only one map, from pseudo to real using opName (actually PseudoInstr, which is derived from opName) as the key. Other maps such as getDPPOp32 also use opName as the key. So if we always wanted to conveniently map two pseudos to one real, we would likely want to introduce several new keys that don't interfere with each other.



================
Comment at: llvm/lib/Target/AMDGPU/VOP3Instructions.td:300
+    defm V_MAD_U64_U32_strict : VOP3Inst <"v_mad_u64_u32", VOP3b_I64_I1_I32_I32_I64>;
+    defm V_MAD_I64_I32_strict : VOP3Inst <"v_mad_i64_i32", VOP3b_I64_I1_I32_I32_I64>;
+  }
----------------
mbrkusanin wrote:
> Joe_Nash wrote:
> > For other types of instructions (VOPC, VOP2) opName is used as a key into mapping tables, so having duplicate entries with the same name can cause collisions. As long as there are no functional issues, I'm fine with it as is. It will be a tablegen failure if someone tries to add a mapping table and use that as the key in the future.
> Is current solutions preferable to having two versions for Real instruction? Problem with having two Reals is we would need to disable one for decoding because of conflict and then manually adjust it, which does not look nice. Encoding is easily resolved with just some extra predicates.
I don't quite understand what you are asking. Currently we have 2 pseudos and one real for each subtarget. We should not need multiple reals on a single subtarget. The way to resolve my comment fully is to go to the following. Which is NFC currently.

defm V_MAD_U64_U32_strict : VOP3Inst <"v_mad_u64_u32**_strict**", VOP3b_I64_I1_I32_I32_I64>;
defm V_MAD_I64_I32_strict : VOP3Inst <"v_mad_i64_i32**_strict**", VOP3b_I64_I1_I32_I32_I64>;


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

https://reviews.llvm.org/D133012