[PATCH] D39415: [ARMISelLowering] Better handling of NEON load/store for sequential memory regions

[Renato Golin via Phabricator via llvm-commits]

rengolin added reviewers: rovka, evandro, mcrosier, kristof.beyls.
rengolin added a comment.

So, this is an interesting change, on the TODO list for a while, but the code and the tests are changing too much in areas where not clearly related.

Furthermore, you have replaced a single-pass for a multi-pass on the Dag, which may have several unintended consequences, including compile time increase.

For your results, you have not shown which is your matrix benchmark, numbers on standard compiler benchmarks and the LLVM test-suite, nor you have said which architecture (aarch32-variant) you're benchmarking.

Giving that this will affect *all* AArch32 (armv7/8), I think we need to be cautious and extensive on our approach.

I'm adding more Arm folks to get a wider review as well as testing, but I am worried with the compilation time as well as the impact in many other code patterns than just matrix multiply.

cheers,
--renato



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11064
+
+  while (true) {
+    for (SDNode::use_iterator UI = Addr.getNode()->use_begin(),
----------------
I'm worried about this... there should be a guaranteed exit condition here.


================
Comment at: test/CodeGen/ARM/cascade-vld-vst.ll:29
+; CHECK:       vld1.32 {d16, d17}, [r0]!
+; CHECK-NEXT:  vld1.32 {d18, d19}, [r0]!
+; CHECK-NEXT:  vld1.32 {d20, d21}, [r0]!
----------------
Use regexp as above:

    {{{d[0-9]+}}, {{d[0-9]+}}}


================
Comment at: test/CodeGen/ARM/misched-fusion-aes.ll:79
 ; CHECK-NEXT: aesmc.8 {{q[0-9][0-9]?}}, [[QC]]
+; CHECK: aese.8 {{q[0-9][0-9]?}}, {{q[0-9][0-9]?}}
 ; CHECK: aese.8 [[QD:q[0-9][0-9]?]], {{q[0-9][0-9]?}}
----------------
why would these change?


================
Comment at: test/CodeGen/ARM/vector-load.ll:258
+; CHECK-NEXT:  vld1.8  {d{{[0-9]+}}}, [r0:64]!
+; CHECK-NEXT:  ldr r0, [r0]
+; CHECK-NEXT:  bx  lr
----------------
you can't guarantee r0 as this returns void.


Repository:
  rL LLVM

https://reviews.llvm.org/D39415