[PATCH] D52116: Introduce llvm.loop.parallel_accesses and llvm.access.group metadata.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: docs/LangRef.rst:5472
+situation that the content must be updated which, because metadata is
+immutable by design, would required finding and update all references
+to the access group node.
----------------
update -> updating


================
Comment at: docs/LangRef.rst:5494
+this loop. Instructions that belong to multiple access groups are
+considered having this property of at least one of the access groups
+matches the ``llvm.loop.parallel_accesses`` list.
----------------
of -> if


================
Comment at: include/llvm/Analysis/VectorUtils.h:120
 
+/// Compute the union of two access group lists.
+///
----------------
access group lists -> access-group lists


================
Comment at: include/llvm/Analysis/VectorUtils.h:126
+
+/// Compute the access group list of access groups that @p Inst1 and @p Inst2 are both in. If either instruction does not access memory at all, it is considered to be in every list.
+///
----------------
access group list -> access-group list


================
Comment at: lib/Analysis/LoopInfo.cpp:328
+            MDNode *AccGroup = cast<MDNode>(AccessListItem.get());
+            assert(AccGroup->isDistinct());
+            assert(AccGroup->getNumOperands() == 0);
----------------
Repeating this set of asserts seems unfortunate. Also, they have no comment. Maybe make a function?

      assert(AccGroup->isDistinct() && "Access group metadata nodes must be distinct");
      assert(AccGroup->getNumOperands() == 0 && "Access group metadata nodes must have zero operands");

you also repeat these asserts in lib/Analysis/VectorUtils.cpp below.


================
Comment at: lib/Analysis/VectorUtils.cpp:474
+    List.insert(AccGroups);
+	return;
+  } 
----------------
Indentation here is odd.


================
Comment at: lib/Analysis/VectorUtils.cpp:477
+
+    for (auto &AccGroupListOp : AccGroups->operands()) {
+      auto* Item = cast<MDNode>(AccGroupListOp.get());
----------------
Indentation here looks odd too.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52116