[PATCH] D80886: [PHIElimination] Fix the killed flag for LowerPHINode()

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/PHIElimination.cpp:327
 
-      // When we are reusing the incoming register, it may already have been
-      // killed in this block. The old kill will also have been inserted at
-      // AfterPHIsIt, so it appears before the current PHICopy.
-      if (reusedIncoming)
-        if (MachineInstr *OldKill = VI.findKill(&MBB)) {
-          LLVM_DEBUG(dbgs() << "Remove old kill from " << *OldKill);
-          LV->removeVirtualRegisterKilled(IncomingReg, *OldKill);
-          LLVM_DEBUG(MBB.dump());
+      MachineInstr *OldKill = VI.findKill(&MBB);
+      bool IsPHICopyAfterOldKill = false;
----------------
This findKill was guarded by `if (reusedIncoming)` in the past. I guess it won't hurt to keep it that way (to avoid any unpleasant surprises with build speeds, not that I think it will cause any big problems but OldKill is only of interest if reusedIncoming is true).


================
Comment at: llvm/test/CodeGen/PowerPC/phi-eliminate.mir:174
   ; CHECK:   %38:g8rc_and_g8rc_nox0 = COPY killed %59
-  ; CHECK:   %37:gprc = COPY killed %57
+  ; CHECK:   %37:gprc = COPY %57
   ; CHECK:   %36:gprc = COPY killed %58
----------------
Could be seen as an unrelated comment, but I've always wondered why PHIElimination swap the order of the defs when converting PHIs to COPYs. I guess it could be easier to deal with the liveness attributes such as "killed" is not reversing the order (but I guess those target hooks, that makes it impossible to know where the COPY is inserted, would still be a nuisance).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80886