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

[Malhar via Phabricator via llvm-commits]

malharJ added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:960-966
+      std::string LocText = " Memory location is the same as accessed at line ";
+      if (SourceLoc) {
+        LocText += std::to_string(SourceLoc.getLine()) + ":" +
+                   std::to_string(SourceLoc.getCol());
+      } else {
+        LocText += "<unknown>:<unknown>";
+      }
----------------
alban.bridonneau wrote:
> malharJ wrote:
> > alban.bridonneau wrote:
> > > We should be able to reuse existing functions to create the full location string. How about using DebugLoc::print?
> > I agree, but I tried DebugLoc::print() and it ends up printing the filename (which is redundant since it is already present at the beginning of the remark), followed by line and column.
> > 
> > Do we want the filename to appear for each debug location we output ?
> > If so, then what should the format of the output messages be ?
> Yes, we want the filename to appear on both locations. The second access could have come from another file, through inlining.
> It might become less readable if the path is really long, but that's something that the user will deal with anyway, if they have long file paths.
Ok, I've used something more standardized now to print out the debug location.


================
Comment at: llvm/test/Transforms/LoopVectorize/loopvectorize-opt-remarks.ll:37
+; // a) Dependence::NoDep
+; // Loop containing only reads does not hinder vectorization
+; void test_nodep(int n, int* A, int* B, int* C) {
----------------
alban.bridonneau wrote:
> I am not sure i understand this test. The description says the loop contains only reads, but the IR has stores in it.
> Also, the IR is already vectorized, so it's not really a useful test case
My bad here. 
Updated the patch with the scalar version for the three cases (nodep, forward, backwardVectorizable)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108371