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

[Alban Bridonneau via Phabricator via llvm-commits]

alban.bridonneau added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:312
+  /// Unsafe memory dependences collected during the analysis
+  /// Used by for OptRemark generation.
+  SmallVector<Dependence, 2> UnsafeDependences;
----------------
Seems you only wanted one of these 2 words?


================
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>";
+      }
----------------
We should be able to reuse existing functions to create the full location string. How about using DebugLoc::print?


================
Comment at: llvm/test/Transforms/LoopVectorize/loopvectorize-opt-remarks.ll:13
+
+define dso_local void @test_unknown_bounds(i32* nocapture %A, i32* nocapture readonly %B, i32 %n) local_unnamed_addr #0 !dbg !13 {
+entry:
----------------
Can we reduce the unit tests to only what's required?
I believe the function attributes like #0 are not required. Neither are the calls to llvm.dbg.value. That should also help to reduce the massive amount of metadata at the bottom of the file.


================
Comment at: llvm/test/Transforms/LoopVectorize/loopvectorize-opt-remarks.ll:23
+for.body.preheader:                               ; preds = %entry
+  %wide.trip.count = zext i32 %n to i64, !dbg !26
+  br label %for.body, !dbg !28
----------------
This kind of line can also be cleaned. Keep the loop control minimal, so that we can focus on the memory dependencies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108371