[GlobalIsel] Handle carry output generated in narrowScalarAddSub legalize action

[yassingh]

Where should I add the tests for this change?

[foad]

In D137407#3907766, @yassingh wrote:

Where should I add the tests for this change?

Does AMDGPU use this code? If so you could put something in test/CodeGen/AMDGPU/GlobalISel/legalize-add.mir.

[yassingh]

In D137407#3908174, @foad wrote:

In D137407#3907766, @yassingh wrote:

Where should I add the tests for this change?

Does AMDGPU use this code? If so you could put something in test/CodeGen/AMDGPU/GlobalISel/legalize-add.mir.

It doesn't. I am exploring the idea of legalizing wide G_UADDO instruction using this narrowScalarAddSub legalize action(as of now it is lowered to G_ADD first) and noticed it does not handle carry generated.

[arsenm]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

5128–5131

Changing the uses from here isn't the way the legalizer expects to work. Each instruction is legalized on its own without modifying other instructions, and using intermediate cast operations. The artifact combiner is then responsible for cleaning out illegal intermediate types

[yassingh]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

5128–5131

The artifact combiner is not automatically doing this. Can you direct where or how should I modify ArtifactCombiner?

[arsenm]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

5128–5131

Re-reading the comment, I think I misunderstood what you were trying to do. In any case, it's inappropriate to be looking through use_instructions here. You should only be modifying the original MI. If you're trying to only modify the one use in MI, you should change that operand directly

[yassingh]

Realized my mistake the carry output is properly updated (if condition, line 5104)

[yassingh]

• New changes abandon carry handling
• updated tests for updated rule
• clang format

[yassingh]

Sorry! Accidently reopened the review with another patch I'm working on. Closing again.

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

for (int i = 0, e = Src1Regs.size(); i != e; ++i) {
 
     DstRegs.push_back(DstReg);
     CarryIn = CarryOut;
   }
   insertParts(MI.getOperand(0).getReg(), RegTy, NarrowTy,
               makeArrayRef(DstRegs).take_front(NarrowParts), LeftoverTy,
               makeArrayRef(DstRegs).drop_front(NarrowParts));
 
+  // Replace the usage of carry genated in original instruction
+  // with carry output generated after sequence of additions
+  auto &MF = *MI.getParent()->getParent();
+  const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
+  for (MachineInstr &Use : MRI.use_instructions(CarryDst)) {
+    Use.substituteRegister(CarryDst, CarryIn, 0, *TRI);
+  }
+
   MI.eraseFromParent();
   return Legalized;
 }
 
 LegalizerHelper::LegalizeResult
 LegalizerHelper::narrowScalarMul(MachineInstr &MI, LLT NarrowTy) {
   Register DstReg = MI.getOperand(0).getReg();
   Register Src1 = MI.getOperand(1).getReg();