[PATCH] D120428: [AArch64] Optimize safe integer division

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Is this a part of AArch64MIPeepholeOpt because it relies upon DIV being ifcvt'd? These peephole optimisations seem notoriously difficult to get right, but it makes sense that you would have to do it so late. We'll just need to be sure it's tested well.



================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:319
+
+  if (CondCode != AArch64CC::EQ || FlagsReg != AArch64::NZCV) {
+    return false;
----------------
This needn't add brackets around single statements according to the llvm code style


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:325
+  auto *RhsDef = MRI->getUniqueVRegDef(RhsReg);
+  auto *FlagsDef = MRI->getUniqueVRegDef(FlagsReg);
+
----------------
Isn't FlagsReg AArch64::NZCV? How does that work with getUniqueVRegDef?


================
Comment at: llvm/lib/Target/AArch64/AArch64MIPeepholeOpt.cpp:473
 
+  LLVM_DEBUG(dbgs() << "********** AArch64 Peephole Optimizer **********\n"
+                    << "********** Function: " << MF.getName() << '\n');
----------------
I don't tend to find these very useful. Does it need to be added?


================
Comment at: llvm/test/CodeGen/AArch64/checked-int-div.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-gnu-linux -mcpu=neoversen1 -o - | FileCheck %s
+
----------------
neoversen1 isn't a valid cpu. Does this not work in other cases due to the costs of div being too high to ifcvt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120428