[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:11077
+/// t2WhileLoopSetup and t2WhileLoopStart to generate WLS loop
+static Register genTPEntry(MachineBasicBlock *MBB_TP_entry,
+                           MachineBasicBlock *MBB_TP_ph,
----------------
Can you look into all these clang-tidy errors. LLVM usually uses CamelCase for variable names, and the MBB_ looks a little odd. They should start with a capital and I would drop the "t2", that's not adding much.


================
Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:6873
+let usesCustomInserter = 1, hasNoSchedulingInfo = 1 in {
+  def MVE_MEMCPYLOOPINST : PseudoInst<(outs),
+                                      (ins rGPR
----------------
Can you improve this formatting. If you clang-formatted it, it doesn't do well with .td files and manually making it look more like the others will do better.


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/memcall.ll:21
+; CHECK-NEXT:    lsrs r7, r7, #4
+; CHECK-NEXT:    wlstp.8 lr, r7, .LBB0_3
+; CHECK-NEXT:    b .LBB0_4
----------------
Why does this not use r3 directly?


================
Comment at: llvm/test/CodeGen/Thumb2/mve_tp_loop.ll:35
+; CHECK-NEXT:    push {r7, lr}
+; CHECK-NEXT:    add.w r3, r2, #15
+; CHECK-NEXT:    bic r3, r3, #16
----------------
What is this generating r3 for? I thought those should be removed.


================
Comment at: llvm/test/CodeGen/Thumb2/mve_tp_loop.ll:55
+; void test3(int* restrict X, int* restrict Y, int n){
+;     printf("n = %d\n", n);
+;     memcpy(X, Y, n);
----------------
Why is this using printf? It looks like an execution test, not a unit test. Is it testing anything specifically? If so it can probably use any call, not a variadic version of printf.


================
Comment at: llvm/test/CodeGen/Thumb2/mve_tp_loop.ll:63
+
+define hidden void @test3(i32* noalias nocapture %X, i32* noalias nocapture readonly %Y, i32 %n) local_unnamed_addr #0 {
+; CHECK-LABEL: test3:
----------------
Remove hidden and local_unnamed_addr #0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99723