[X86][MC] Avoid emitting incorrect warning for complex FMUL

[pengfei]

Summary

We will insert a new operand which is identical to the Dest for complex
FMUL with a mask. https://godbolt.org/z/eTEdnYv3q

Complex FMA and FMUL with maskz don't have this problem.

[RKSimon]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3851

comment?

[pengfei]

Add comment.

[craig.topper]

I would replace the word "fake" in the title with "incorrect".

And "to emit" change to "emitting"

[LuoYuanke]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3853

Just be curious. Since there is not issue for VFCMADDCPH, why for VFCMULCPH the mask version does not conform with non-mask version?

[skan]

In D130638#3682681, @craig.topper wrote:

I would replace the word "fake" in the title with "incorrect".

And "to emit" change to "emitting"

"to emitting" -> "emitting"

[pengfei]

In D130638#3683989, @skan wrote:

In D130638#3682681, @craig.topper wrote:

I would replace the word "fake" in the title with "incorrect".

And "to emit" change to "emitting"

"to emitting" -> "emitting"

🤦‍♀️

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3853

The reason is in the differences of the input dag (generated by -print-records):

def VFCMADDCPHZr {
... ...
  dag OutOperandList = (outs VR512:$dst);
  dag InOperandList = (ins VR512:$src1, VR512:$src2, VR512:$src3);
... ...
  string Constraints = "@earlyclobber $dst, $src1 = $dst";
... ...
}
def VFMULCPHZrr {
... ...
  dag OutOperandList = (outs VR512:$dst);
  dag InOperandList = (ins VR512:$src1, VR512:$src2);
... ...
  string Constraints = "@earlyclobber $dst";
... ...
}
def VFMULCPHZrrk {
... ...
  dag OutOperandList = (outs VR512:$dst);
  dag InOperandList = (ins VR512:$src0, VK16WM:$mask, VR512:$src1, VR512:$src2);
... ...
  string Constraints = "@earlyclobber $dst, $src0 = $dst";
... ...
}

[LuoYuanke]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3853

Thanks, Phoebe.

[LuoYuanke]

LGTM, thanks.

[skan]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3853

Do we need X86II:EVEX_Z here?

3854

Using condtional operator here could be more clear

unsigned i = (TSFlags & X86II::EVEX_Z) ? 2: 1;

[pengfei]

Address Shengchen's comments. Thanks!

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

3853

Yeah, it's safe to remove it.

[skan]

LGTM

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

if (isVFCMADDCPH(Opcode) || isVFCMADDCSH(Opcode) || isVFMADDCPH(Opcode) ||
     unsigned Dest = Inst.getOperand(0).getReg();
     for (unsigned i = 2; i < Inst.getNumOperands(); i++)
       if (Inst.getOperand(i).isReg() && Dest == Inst.getOperand(i).getReg())
         return Warning(Ops[0]->getStartLoc(), "Destination register should be "
                                               "distinct from source registers");
   } else if (isVFCMULCPH(Opcode) || isVFCMULCSH(Opcode) || isVFMULCPH(Opcode) ||
              isVFMULCSH(Opcode)) {
     unsigned Dest = Inst.getOperand(0).getReg();
-    for (unsigned i = 1; i < Inst.getNumOperands(); i++)
+    // The mask variants have different operand list. Scan from the third
+    // operand to avoid emitting incorrect warning.
+    //    VFMULCPHZrr   Dest, Src1, Src2
+    //    VFMULCPHZrrk  Dest, Dest, Mask, Src1, Src2
+    //    VFMULCPHZrrkz Dest, Mask, Src1, Src2
+    for (unsigned i = TSFlags & X86II::EVEX_K ? 2 : 1;
+         i < Inst.getNumOperands(); i++)
       if (Inst.getOperand(i).isReg() && Dest == Inst.getOperand(i).getReg())
         return Warning(Ops[0]->getStartLoc(), "Destination register should be "
                                               "distinct from source registers");
   } else if (isV4FMADDPS(Opcode) || isV4FMADDSS(Opcode) ||
              isV4FNMADDPS(Opcode) || isV4FNMADDSS(Opcode) ||
              isVP4DPWSSDS(Opcode) || isVP4DPWSSD(Opcode)) {
     unsigned Src2 = Inst.getOperand(Inst.getNumOperands() -
                                     X86::AddrNumOperands - 1).getReg();

llvm/test/MC/X86/avx512fp16.s

 // RUN: llvm-mc -triple x86_64-unknown-unknown --show-encoding < %s  | FileCheck %s
+// RUN: llvm-mc -triple x86_64-unknown-unknown < %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO-WARN
+
+// CHECK-NO-WARN-NOT: warning
 
 // CHECK: vmovsh %xmm28, %xmm29, %xmm30
 // CHECK: encoding: [0x62,0x05,0x16,0x00,0x10,0xf4]
           vmovsh %xmm28, %xmm29, %xmm30
 
 // CHECK: vmovsh 268435456(%rbp,%r14,8), %xmm30 {%k7}
 // CHECK: encoding: [0x62,0x25,0x7e,0x0f,0x10,0xb4,0xf5,0x00,0x00,0x00,0x10]
           vmovsh 268435456(%rbp,%r14,8), %xmm30 {%k7}