[PATCH] D24934: [LICM] Add support of a new optimization case to Loop Versioning for LICM + code clean up

[Adam Nemet via llvm-commits]

anemet added inline comments.


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:664
+        } else {
+          CanDoRT = false;
+        }
----------------
eastig wrote:
> sbaranga wrote:
> > Won't this just make canCheckPtrAtRT say that it can ignore all pointers with unknown bounds?
> > I think this would be incorrect for all other LAA users.
> > 
> > 
> They are not ignored. They are collected to be post-processed.
> 
> The default behaviour is not to create any RT checks if a pointer with unknown bounds has been found.  I think this is too strict. What if I don't want to discard recognized pointers and RT checks for them. Any idea how to implement this without duplicating the code?
> 
> The default behaviour is not changed if a LAA user does not want to have information about pointers with unknown bounds. I don't how the default behaviour can be changed unintentionally.
> 
> LAA was originally developed for the Loop Vectorizer. But now it is suggested to be used as the loop memory dependence framework. The problem is that the Loop Vectorizer specifics are not removed. The current users of LLA, besides the Loop Vectorizer, are LoopDistribute and LoopLoadElimination passes. They are also for vectorizing loops.
It would be good if you could describe what behavior you're trying to achieve.

If I remember correctly both LDist and LLE post-process the full set of run-time checks, so you probably want something similar.  (Sorry if that is what you're doing.  I haven't looked at the patch because it has many unrelated changes.)

> LAA was originally developed for the Loop Vectorizer. But now it is suggested to be used as the loop memory dependence framework. The problem is that the Loop Vectorizer specifics are not removed. 

This is not accurate and it's misleading. Yes, the LV-specific APIs were retained but new generic API were developed.  The LV-specific APIs are now typically formulated in terms of the generic APIs.  It was done this way to allow incremental migration.  If the LV-specific APIs are misleading we could move them to LV.



https://reviews.llvm.org/D24934