[PATCH] D70634: Ignore Unknown dependencies using vectorize.ivdep metadata

Mon Nov 25 09:01:41 PST 2019

Meinersbur added a comment.

[serious] We did not come to a conclusion during the discussion <https://lists.llvm.org/pipermail/llvm-dev/2019-August/134592.html> about the semantics ivdep being implementation-dependent.

Also consider that `vectorize_enable(assume_safety)` has been designed such that if previous passes move code into the loop that has not been in the source code loop, the safety semantic does not apply to the moved code. This implementation would apply ivdep on the moved code (which might be okay if we shift the responsibility to the code motion pass to remove the metadata).

================
Comment at: llvm/docs/LangRef.rst:5492

+'``llvm.loop.vectorize.ivdep.enable``' Metadata
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
[serious] Please also describe the relationship to `llvm.mem.parallel_loop_access`.

================
Comment at: llvm/include/llvm/Analysis/LoopAccessAnalysis.h:204
   bool areDepsSafe(DepCandidates &AccessSets, MemAccessInfoList &CheckDeps,
-                   const ValueToValueMap &Strides);
+                   const ValueToValueMap &Strides, bool ignUnDep);

----------------
[style] [[ https://www.llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | Start variables with a capital letter ]]. Also to not use two different styles on the same line.

================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:2041-2043
+    CanVecMemIvdep = DepChecker->areDepsSafe(
+        DependentAccesses, Accesses.getDependenciesToCheck(), SymbolicStrides,
+        true);
----------------
[serious] `areDepsSafe` is an expensive call which we should not do redundantly when only one of the result is actually used.

[suggestion] Use only `CanVecMem` and update it according to ivdep.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70634