[PATCH] D133103: [PowerPC] Improve kill flag computation and add verification after MI peephole

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:379
+                                   Register Dst
+                                   ) {
   DenseMap<MachineInstr *, MachineInstr *> ChangedPHIMap;
----------------
nit: spacing


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:431
+      if (MRI->isSSA())
+        addRegToUpdate(RegMBB.first.getReg());
+    }
----------------
We are adding `RegMBB.first` but what about `RegMBB.second` and `AccReg`? This is a new instruction so I assume that all of these registers now may have a different live range.


================
Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1927
   SrcMI->eraseFromParent();
+  addRegToUpdate(NewInstr->getOperand(1).getReg());
   return true;
----------------
There are two instructions that are erased here and I don't think all of the required registers are added to the update list. 
I managed to get a reduced test case that I think shows this issue:
```
define hidden void @testCaller() local_unnamed_addr align 2 {
entry:
  br label %exit

exit: ; preds = %entry
  br label %cond.end.i.i

cond.end.i.i:                                     ; preds = %if.end116, %exit
  %CurrentState.0566 = phi i32 [ %CurrentState.2, %if.end116 ], [ -1, %exit ]
  %0 = load i32, ptr poison, align 8
  br label %while.body5.i

while.cond12.preheader.i:                         ; preds = %while.body5.i
  br i1 poison, label %if.end116, label %for.cond99.preheader

while.body5.i:                                    ; preds = %while.body5.i, %cond.end.i.i
  %Test.012.i = phi i32 [ 0, %cond.end.i.i ], [ %dec10.i, %while.body5.i ]
  %dec10.i = add nsw i32 %Test.012.i, -1
  %cmp4.i = icmp slt i32 0, %dec10.i
  br i1 %cmp4.i, label %while.body5.i, label %while.cond12.preheader.i

for.cond99.preheader:                             ; preds = %while.cond12.preheader.i
  %1 = load ptr, ptr poison, align 8
  %conv103 = sext i32 %0 to i64
  %arrayidx.i426 = getelementptr inbounds i32, ptr %1, i64 %conv103
  store i32 0, ptr %arrayidx.i426, align 4
  store i32 %CurrentState.0566, ptr poison, align 8
  br label %if.end116

if.end116:                                        ; preds = %for.cond99.preheader, %while.cond12.preheader.i
  %CurrentState.2 = phi i32 [ %0, %while.cond12.preheader.i ], [ poison, %for.cond99.preheader ]
  call fastcc void @callee()
  br label %cond.end.i.i
}
declare dso_local fastcc void @callee() unnamed_addr align 2
```
Compile with:
```
llc --mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr9 -debug-only=ppc-mi-peepholes bugpoint-reduced-simplified.ll
```

I see the following output:
```
Combining pair:   %2:g8rc = EXTSW_32_64 %1:gprc
  %12:g8rc = RLDICR killed %2:g8rc, 2, 61
TO:   %12:g8rc = EXTSWSLI_32_64 %1:gprc, 2
Adding register: 1 on line 1944 for re-computation of kill flags
Deleting instruction:   %12:g8rc = RLDICR killed %2:g8rc, 2, 61
...
*** Bad machine code: LiveVariables: Block should not be in AliveBlocks ***
- function:    testCaller
- basic block: %bb.2 while.cond12.preheader.i (0x119473888)
Virtual register %2 is not needed live through the block.

*** Bad machine code: LiveVariables: Block should not be in AliveBlocks ***
- function:    testCaller
- basic block: %bb.3 while.body5.i (0x119473988)
Virtual register %2 is not needed live through the block.
LLVM ERROR: Found 2 machine code errors.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133103/new/

https://reviews.llvm.org/D133103