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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11064
+
+  while (true) {
+    for (SDNode::use_iterator UI = Addr.getNode()->use_begin(),
----------------
rengolin wrote:
> evgeny777 wrote:
> > rengolin wrote:
> > > I'm worried about this... there should be a guaranteed exit condition here.
> > What about reducing number of iterations to, let's say, 16? This should be more than enough for most of cases. Also the effect of compilation time increase should become limited.
> I was hoping for an approach that didn't need O(n^2) even if lower bound. If your increment is 16x, then the loop will be 16x less efficient than previously. Granted, it's doing more, but that's a big impact that I'd rather avoid.
> 
> But I have been away from isel for quite a while now, so I'll wait for some current isel developers (@qcolombet @rovka) to shed some light.
I'll have to think about this a bit more... but I'll just briefly note that the existing CombineBaseUpdate has shown up in the past as a compile-time hog for large basic blocks.  (I think it's worst-case O(N^3) or something like that.)


Repository:
  rL LLVM

https://reviews.llvm.org/D39415