[PATCH] D99723: [ARM] Transforming memcpy to Tail predicated Loop

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:11257
+    //          |                 |
+    //          |         TP loop Body MBB
+    //          \                |
----------------
Great comment by the way. Is it possible to make the loop MBB look like a bit like a loop? To show there is a backedge too.


================
Comment at: llvm/lib/Target/ARM/ARMSelectionDAGInfo.cpp:140
+
+  auto GenInlineTP = [=](const ARMSubtarget &Subtarget,
+                         const SelectionDAG &DAG) {
----------------
`[=]` ->`[&]` is more standard, even if it doesn't make a lot of difference here.


================
Comment at: llvm/lib/Target/ARM/ARMSelectionDAGInfo.cpp:142
+                         const SelectionDAG &DAG) {
+    return !DAG.getMachineFunction().getFunction().hasOptNone() &&
+           ((!ConstantSize && (Alignment >= Align(4))) ||
----------------
Probably better as:
```
if (DAG.getMachineFunction().getFunction().hasOptNone())
  return false;
if (!ConstantSize && (Alignment >= Align(4))
  return true;
if (...)
 ...
```

The EnableMemcpyTPLoop  logic could be in here too, as it's just returning true/false at the right time.
What do we do for -Oz and -Os?


================
Comment at: llvm/test/CodeGen/Thumb2/mve_tp_loop.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O1 -mtriple=arm-arm-none-eabi -mcpu=cortex-m55 --verify-machineinstrs %s -o - | FileCheck %s
+
----------------
Shouldn't have -O1 or cpu, use the -mtriple from other similar tests. The test can be called llvm/test/CodeGen/Thumb2/mve-tp-loop.ll.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99723