[X86] Apply the new instruction's register classes constraints on the operands of the replaced instruction when memory folding

[aymanmus]

Summary

When performing memory folding (thus changing the register form of an instruction to it's memory form), make sure the register classes of the machine operands of the replaced instruction comply to the constrains of the register classes of the new instruction - If not, change them.

[craig.topper]

Test case?

[RKSimon]

lib/Target/X86/X86InstrInfo.cpp

7713

for (unsigned i = 0, e = MOs.size(); i < e; i++) {

[aymanmus]

@craig.topper, i encountered this issue by coincidence but could not find a test case which fails without this. all the folded instructions right now have the same register classes constrains for both register and memory forms.
But as we entirely replace the instruction I saw that this change should be added even though it doesn't effect anything right now. Maybe it will be needed in the future.

[aymanmus]

Ping.

[RKSimon]

In D32680#744574, @aymanmus wrote:

@craig.topper, i encountered this issue by coincidence but could not find a test case which fails without this. all the folded instructions right now have the same register classes constrains for both register and memory forms.
But as we entirely replace the instruction I saw that this change should be added even though it doesn't effect anything right now. Maybe it will be needed in the future.

Will test cases appear after D32684 is applied?

[aymanmus]

@RKSimon, Still no.

[aymanmus]

Abandoning - lack of response.

lib/Target/X86/X86InstrInfo.cpp

for (unsigned i = 0; i != NumAddrOps; ++i) {
         MIB.addDisp(MO, PtrOffset);
       } else {
         MIB.add(MO);
       }
     }
   }
 }
 
+static void ApplyRegClassConstraints(MachineFunction &MF,
+                                     const MCOperandInfo &OpInfo,
+                                     const MachineOperand &MO) {
+  const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
+  if (MO.isReg() && TargetRegisterInfo::isVirtualRegister(MO.getReg())) {
+    const TargetRegisterClass *RC;
+    uint8_t RegClass = OpInfo.RegClass;
+    if (OpInfo.isLookupPtrRegClass())
+      RC = TRI->getPointerRegClass(MF, RegClass);
+    else
+      RC = TRI->getRegClass(RegClass);
+
+    MF.getRegInfo().constrainRegClass(MO.getReg(), RC);
+  }
+}
+
 static MachineInstr *FuseTwoAddrInst(MachineFunction &MF, unsigned Opcode,
                                      ArrayRef<MachineOperand> MOs,
                                      MachineBasicBlock::iterator InsertPt,
                                      MachineInstr &MI,
                                      const TargetInstrInfo &TII) {
   // Create the base instruction with the memory operand as the first part.
   // Omit the implicit operands, something BuildMI can't do.
   MachineInstr *NewMI =
       MF.CreateMachineInstr(TII.get(Opcode), MI.getDebugLoc(), true);
   MachineInstrBuilder MIB(MF, NewMI);
+
+  // Apply the new instruction's register classes constraints.
+  for (unsigned i = 0, e = MOs.size(); i < e; i++) {
+    const MCOperandInfo &OpInfo = NewMI->getDesc().OpInfo[i];
+    ApplyRegClassConstraints(MF, OpInfo, MOs[i]);
+  }
+
   addOperands(MIB, MOs);
 
   // Loop over the rest of the ri operands, converting them over.
   unsigned NumOps = MI.getDesc().getNumOperands() - 2;
   for (unsigned i = 0; i != NumOps; ++i) {
     MachineOperand &MO = MI.getOperand(i + 2);
+
+    // Apply the new instruction's register classes constraints.
+    const MCOperandInfo &OpInfo = NewMI->getDesc().OpInfo[i + MOs.size() + 1];
+    ApplyRegClassConstraints(MF, OpInfo, MO);
+
     MIB.add(MO);
   }
   for (unsigned i = NumOps + 2, e = MI.getNumOperands(); i != e; ++i) {
     MachineOperand &MO = MI.getOperand(i);
     MIB.add(MO);
   }
 
   MachineBasicBlock *MBB = InsertPt->getParent();
   MBB->insert(InsertPt, NewMI);
 
   return MIB;
 }
 
 static MachineInstr *FuseInst(MachineFunction &MF, unsigned Opcode,
                               unsigned OpNo, ArrayRef<MachineOperand> MOs,
                               MachineBasicBlock::iterator InsertPt,
                               MachineInstr &MI, const TargetInstrInfo &TII,
                               int PtrOffset = 0) {
   // Omit the implicit operands, something BuildMI can't do.
   MachineInstr *NewMI =
       MF.CreateMachineInstr(TII.get(Opcode), MI.getDebugLoc(), true);
   MachineInstrBuilder MIB(MF, NewMI);
 
+  unsigned AddIdx = 0;
   for (unsigned i = 0, e = MI.getNumOperands(); i != e; ++i) {
     MachineOperand &MO = MI.getOperand(i);
     if (i == OpNo) {
       assert(MO.isReg() && "Expected to fold into reg operand!");
+
+      // Apply the new instruction's register classes constraints.
+      for (unsigned j = 0, e = MOs.size(); j < e; j++) {
+        const MCOperandInfo &OpInfo = NewMI->getDesc().OpInfo[i + j];
+        ApplyRegClassConstraints(MF, OpInfo, MOs[j]);
+      }
+
       addOperands(MIB, MOs, PtrOffset);
+      AddIdx = MOs.size();
     } else {
+      const MCOperandInfo &OpInfo = NewMI->getDesc().OpInfo[i + AddIdx];
+      ApplyRegClassConstraints(MF, OpInfo, MO);
       MIB.add(MO);
     }
   }
 
   MachineBasicBlock *MBB = InsertPt->getParent();
   MBB->insert(InsertPt, NewMI);
 
   return MIB;