AMDGPU: Use OtherPredicates for v_dot2_bf16_bf16(f16_f16) pseudo

[changpeng]

This is because SubtargetPredicate is not copied from pseudo to dpp16 and dpp8 real. Actually this is the common issue for insts with Realtriple --- We should avoid using SubtargetPredicate to define pseudo: the predicate will be lost in real.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Changpeng Fang (changpeng)

Changes

This is because SubtargetPredicate is not copied from pseudo to dpp16 and dpp8 real. Actually this is the common issue for insts with Realtriple --- We should avoid using SubtargetPredicate to define pseudo: the predicate will be lost in real.

---

Full diff: https://github.com/llvm/llvm-project/pull/84354.diff

1 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/VOP3Instructions.td (+1-1)

diff --git a/llvm/lib/Target/AMDGPU/VOP3Instructions.td b/llvm/lib/Target/AMDGPU/VOP3Instructions.td
index 334cfad478f151..3340ded9d36000 100644
--- a/llvm/lib/Target/AMDGPU/VOP3Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3Instructions.td
@@ -893,7 +893,7 @@ let SubtargetPredicate = isGFX12Plus, ReadsModeReg = 0 in {
   defm V_MINIMUMMAXIMUM_F16 : VOP3Inst<"v_minimummaximum_f16", VOP3_Profile<VOP_F16_F16_F16_F16, VOP3_OPSEL>>;
 } // End SubtargetPredicate = isGFX12Plus, ReadsModeReg = 0
 
-let SubtargetPredicate = HasDot9Insts, IsDOT=1 in {
+let OtherPredicates = [HasDot9Insts], IsDOT=1 in {
   defm V_DOT2_F16_F16 :   VOP3Inst<"v_dot2_f16_f16",   VOP3_DOT_Profile<VOP_F16_V2F16_V2F16_F16>, int_amdgcn_fdot2_f16_f16>;
   defm V_DOT2_BF16_BF16 : VOP3Inst<"v_dot2_bf16_bf16", VOP3_DOT_Profile<VOP_BF16_V2BF16_V2BF16_BF16>, int_amdgcn_fdot2_bf16_bf16>;
 }

[rampitec]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

[changpeng]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;

I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be
very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

[changpeng]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;

I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need?
let AssemblerPredicate = HasDPP8;

Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough?
"AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

[rampitec]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;
I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need? let AssemblerPredicate = HasDPP8;

Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough? "AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

Imagine you have the same instruction in different targets, so you are using 'SubtargetPredicate = HasFeature' on the pseudo. But the encoding is different, so you this HasFeature is not sufficient, you need to use AssemblerPredicate to distinguish between targets for real instructions.

[changpeng]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;
I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need? let AssemblerPredicate = HasDPP8;
Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough? "AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

Imagine you have the same instruction in different targets, so you are using 'SubtargetPredicate = HasFeature' on the pseudo. But the encoding is different, so you this HasFeature is not sufficient, you need to use AssemblerPredicate to distinguish between targets for real instructions.

Is it redundant to have HasDPP8 in both SubtargetPredicate and AssemblerPredicate for a real?
I think it is SubtargetPredicate && AssemblerPredicate

[rampitec]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;
I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need? let AssemblerPredicate = HasDPP8;
Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough? "AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

Imagine you have the same instruction in different targets, so you are using 'SubtargetPredicate = HasFeature' on the pseudo. But the encoding is different, so you this HasFeature is not sufficient, you need to use AssemblerPredicate to distinguish between targets for real instructions.

Is it redundant to have HasDPP8 in both SubtargetPredicate and AssemblerPredicate for a real? I think it is SubtargetPredicate && AssemblerPredicate

Yes, both should not be needed.

[changpeng]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;
I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need? let AssemblerPredicate = HasDPP8;
Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough? "AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

Imagine you have the same instruction in different targets, so you are using 'SubtargetPredicate = HasFeature' on the pseudo. But the encoding is different, so you this HasFeature is not sufficient, you need to use AssemblerPredicate to distinguish between targets for real instructions.

Is it redundant to have HasDPP8 in both SubtargetPredicate and AssemblerPredicate for a real? I think it is SubtargetPredicate && AssemblerPredicate

Yes, both should not be needed.

Then we can remove "let SubtargetPredicate = ...", and copy SubtargetPredicate from pseudo to real for dpp16 and dpp8.
I am going to look into this. Thanks

[changpeng]

LGTM, but this is really unfortunate. Can we try to address dpp predicates?

let SubtargetPredicate = HasDPP8;
I am thinking whether we can put hasDPP8 into OtherPredicates (using concat). But we need to be very careful of the ordering this assignment with copy from pseudo to real of OtherPredicates.

let SubtargetPredicate = HasDPP8; // do we really need? let AssemblerPredicate = HasDPP8;
Actually why do we need to set both SubtargetPredicate and AssemblerPredicate? One of them enough? "AssemblerPredicate = HasDPP8" may also have "overriden" issue on the path.

Imagine you have the same instruction in different targets, so you are using 'SubtargetPredicate = HasFeature' on the pseudo. But the encoding is different, so you this HasFeature is not sufficient, you need to use AssemblerPredicate to distinguish between targets for real instructions.

Is it redundant to have HasDPP8 in both SubtargetPredicate and AssemblerPredicate for a real? I think it is SubtargetPredicate && AssemblerPredicate

Yes, both should not be needed.

Then we can remove "let SubtargetPredicate = ...", and copy SubtargetPredicate from pseudo to real for dpp16 and dpp8. I am going to look into this. Thanks

#84517