[PowerPC] Remove implicit use register after transformToImmFormFedByLI()

[lkail]

Summary

When the instruction has imm form and fed by LI, we can remove the redundat LI instruction.
Below is an example:

renamable $x5 = LI8 2
renamable $x4 = exact SRD killed renamable $x4, killed renamable $r5, implicit $x5

will be converted to:

renamable $x5 = LI8 2
renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

But when we do this optimization, we forget to remove implicit killed $x5
This bug has caused a lnt case error. This patch is to fix above bug.

[lkail]

Current fix looks not in a proper direction. If I remove lines llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp:[2259-2261) in https://reviews.llvm.org/D56078, the test in this patch gets passed.
I tried dump MI and its explicit_operands, the output looks not what I expected.
Code modification

if (SpecialShift32 || SpecialShift64) {
  MI.dump();
  for (auto &op : MI.explicit_operands()) {
    op.dump();
  }

Output

  renamable $x4 = exact RLDICL killed renamable $x4, killed renamable $r5, implicit $x5
renamable $x4
killed renamable $x4
killed renamable $r5
implicit $x5

implicit $x5 is in the explicit operand list.

[lkail]

Do not early return in PPCInstrInfo::replaceInstrOperandWithImm.

[lkail]

Ping

[shchenz]

implicit $x5 is in the explicit operand list.

Should it be an issue that the implicit $x5 should not be in the explicit operand list? Since when we have implicit $x5 as an explicit operand?

[lkail]

In D85288#2687705, @shchenz wrote:

implicit $x5 is in the explicit operand list.

Should it be an issue that the implicit $x5 should not be in the explicit operand list? Since when we have implicit $x5 as an explicit operand?

Take

renamable $x5 = LI8 2
renamable $x4 = exact SRD killed renamable $x4, killed renamable $r5, implicit $x5

converted to

renamable $x5 = LI8 2
renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

as an example.
SRD has 2 explicit operands. When SRD is converted to RLDICL via setting a new MCID and RLDICL has 3 explicit operands, so implicit $x5 is within RLDICL's explicit operands. Later replaceInstrOperandWithImm is called and killed renamable $r5 is converted to imm 62. With origin impl, this instruction currently is

renamable $x4 = exact RLDICL killed renamable $x4, 62, implicit killed $x5

Then MachineInstrBuilder(*MI.getParent()->getParent(), MI).addImm(ME); is called. Notice that addImm's implementation will traverse the operand list and find a location explicit operand goes before implicit operand, so for now this instruction is

renamable $x4 = exact RLDICL killed renamable $x4, 62, 2,  implicit killed $x5

[shchenz]

Thanks for the detailed explanation. I am thinking the replaceInstrOperandWithImm is functionality complete.
The issue here is checking an empty implicit operand set by MI.implicit_operands().empty() is not right as the MI is in the construction phase after calling setDesc()

Can we add a general function in MachineInstr class to check the empty implicit operand set by iterating all the operands?

[lkail]

Can we add a general function in MachineInstr class to check the empty implicit operand set by iterating all the operands?

Looks not a good idea to check within MachineIstr. See https://reviews.llvm.org/D100453.

[shchenz]

LGTM. Thanks.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

if (III.ZeroIsSpecialNew) {
         const TargetRegisterClass *NewRC =
           MRI.getRegClass(RegToModify)->hasSuperClassEq(&PPC::GPRCRegClass) ?
           &PPC::GPRC_and_GPRC_NOR0RegClass : &PPC::G8RC_and_G8RC_NOX0RegClass;
         MRI.setRegClass(RegToModify, NewRC);
       }
     }
   }
 
+  // Remove the implicit registers defined by DefMI.
+  const TargetRegisterInfo *TRI = &getRegisterInfo();
+  int idx = MI.findRegisterUseOperandIdx(DefMI.getOperand(0).getReg(),
+                                         false, TRI);
+  if (idx != -1 && MI.getOperand(idx).isImplicit()) {
+    // Below is an example:
+    // renamable $x5 = LI8 2
+    // renamable $x4 = exact RLDICL killed renamable $x4, 62, 2, implicit $x5
+    // ==> Remove the implicit $x5
+    // dead renamable $x5 = LI8 2
+    // renamable $x4 = exact RLDICL killed renamable $x4, 62, 2
+    MI.RemoveOperand(idx);
+  }
+
   // Fix up killed/dead flag after transformation.
   // Pattern:
   // ForwardKilledOperandReg = LI imm
   // y = XOP reg, ForwardKilledOperandReg(killed)
   if (ForwardKilledOperandReg != ~0U)
     fixupIsDeadOrKill(DefMI, MI, ForwardKilledOperandReg);
   return true;
 }

llvm/test/CodeGen/PowerPC/remove-redundant-li-implicit-reg.mir

+# RUN: llc -mtriple=powerpc64le--linux-gnu -stop-after ppc-pre-emit-peephole %s -o - -verify-machineinstrs | FileCheck %s
+
+
+---
+name: testRedundantLiImplicitReg
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $x4, $x5
+
+    STW killed $r3, killed $x5, 100
+    renamable $x5 = LI8 2
+    renamable $x4 = exact SRD killed renamable $x4, killed renamable $r5, implicit $x5
+    STD $x4, $x4, 100
+    BLR8 implicit $lr8, implicit $rm
+
+  ; CHECK-LABEL: testRedundantLiImplicitReg
+  ; CHECK: bb.0.entry:
+  ; CHECK:   STW killed $r3, killed $x5, 100
+  ; CHECK:   renamable $x4 = exact RLDICL killed renamable $x4, 62, 2
+  ; CHECK:   STD killed $x4, $x4, 100
+  ; CHECK:   BLR8 implicit $lr8, implicit $rm
+...