[PATCH] D122125: [PowerPC] generate CTR loops instructions after ISEL

[Kai Luo via Phabricator via llvm-commits]

lkail added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCCTRLoops.cpp:107
+bool PPCCTRLoops::isCTRClobber(MachineInstr *MI, bool CheckReads) const {
+  if ((MI->definesRegister(PPC::CTR) && !MI->registerDefIsDead(PPC::CTR)) ||
+      (MI->definesRegister(PPC::CTR8) && !MI->registerDefIsDead(PPC::CTR8)))
----------------
Can we use `modifiesRegsiter` here to further check RegMask clobbering?


================
Comment at: llvm/lib/Target/PowerPC/PPCCTRLoops.cpp:114
+
+  if (MI->getDesc().isCall())
+    return true;
----------------
Why would a `Call` read CTR register?


================
Comment at: llvm/lib/Target/PowerPC/PPCCTRLoops.cpp:153
+  MachineInstr *Dec = nullptr;
+  bool Revert = false;
+
----------------
Can we rename `Revert` to something like `InvalidCTRLoop`? `Revert` sounds like we have performed part of transformation, then rollback.


================
Comment at: llvm/lib/Target/PowerPC/PPCCTRLoops.cpp:157
+  // If there is no preheader for this loop, there must be no MTCTRPseudo
+  // eigher.
+  if (!Preheader)
----------------
Typo: either


================
Comment at: llvm/test/CodeGen/PowerPC/ctrloops64.mir:239
+    BLR implicit $lr, implicit $rm
+...
----------------
Please add test whose loop has calls into other functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122125