[PATCH] D108371: [LAA] Add Memory dependence and unknown bounds remarks.

[Malhar via Phabricator via llvm-commits]

malharJ added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:2074
         CanVecMem = false;
+        FailReason = FailureReason::UnsafeDataDependenceTriedRT;
         return;
----------------
sdesmalen wrote:
> Is this case not already covered by the `recordAnalysis` call above?
> 
> It seems like a mechanism for doing this already exists (recordAnalysis), it may be worth extending that to handle multiple reports (there is currently a limitation that it uses a single `Report` variable, but perhaps that could be made into a vector of reports which can be appended to).
> 
I agree it has been covered by the call to `recordAnalysis()` at line 2070 ... 
I've now moved that call to `recordAnalysis()` to inside `elaborateMemoryReport()`.


Regarding the second issue, I'm not sure myself why `recordAnalysis` has Report as scalar and not a vector but that is not really a part of my change ...  I just tried to re-use it. 


================
Comment at: llvm/test/Analysis/LoopAccessAnalysis/memory-dep-remarks.ll:12
+
+; CHECK: remark: source.c:4:16: cannot identify array bounds
+
----------------
sdesmalen wrote:
> Would a prefix like "Loop cannot be vectorized: " be useful?
> 
> I think the message is a bit cryptic, how about:
> 
>   Unable to determine aliasing information because the bounds of this access cannot be computed.
If you see the earlier version/diffs of the patch, the reporting was being done in LoopVectorizer.
That time I had added "loop not vectorized" in the tests.

But now that it has moved to LoopAccessAnalysis, we are no longer emitting "loop not vectorized" here.
That prefix is prepended by the function reportVectorizationFailure() in LoopVectorize.cpp

For the latter part, I have made the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108371