[X86] Add necessary check isReg() when updating LiveVariables in convertToThreeAddress

[yubing]

I found my reproducer can't work for llvm-project's code.
could we merge it without adding a testcase?

[RKSimon]

llvm/lib/Target/X86/X86InstrInfo.cpp

1772

By inspection - the getReg below will assert if its isn't a register so this should be OK.

@foad might have a suggestion regarding alternative ways to test though?

[foad]

llvm/lib/Target/X86/X86InstrInfo.cpp

1772

The instruction that @yubing cited in D136596 was: %22:vr512 = VPBROADCASTDZrmk %21:vr512(tied-def 0), killed %20:vk16wm, %stack.1, 1, $noreg, 0, $noreg :: (load (s32) from %stack.1, align 64)

So I guess maybe any of these *rmk instructions handled at line 1690 could have an operand that is %stack.n instead of a register?

[yubing]

ping? @RKSimon @foad

[RKSimon]

No success in creating a mir test?

[yubing]

In D137388#3914998, @RKSimon wrote:

No success in creating a mir test?

yeah, i may not have time to create a reproducer.

[RKSimon]

LGTM - by inspection this works to avoid the assert

[RKSimon]

@yubing https://github.com/llvm/llvm-project/issues/58914 might have just found your test case ?

llvm/lib/Target/X86/X86InstrInfo.cpp

MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
   }
   }
 
   if (!NewMI) return nullptr;
 
   if (LV) {  // Update live variables
     for (unsigned I = 0; I < NumRegOperands; ++I) {
       MachineOperand &Op = MI.getOperand(I);
-      if (Op.isDead() || Op.isKill())
+      if (Op.isReg() && (Op.isDead() || Op.isKill()))
         LV->replaceKillInstruction(Op.getReg(), MI, *NewMI);
     }
   }
 
   MachineBasicBlock &MBB = *MI.getParent();
   MBB.insert(MI.getIterator(), NewMI); // Insert the new inst
 
   if (LIS) {
     LIS->ReplaceMachineInstrInMaps(MI, *NewMI);