[PATCH] D52653: [CodeGen, AArch64] Combine Interleaved Loads which are not covered by the Vectorizer

John Brawn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 5 05:04:51 PST 2018


john.brawn accepted this revision.
john.brawn added a comment.
This revision is now accepted and ready to land.

A few errors in comments, but otherwise LGTM so long as you fix those errors before committing.



================
Comment at: include/llvm/CodeGen/Passes.h:383
+  /// InterleavedLoadCombines Pass - This pass identifies interleaved loads and
+  /// combines them in wide loads detectable be InterleavedAccessPass
+  ///
----------------
"combines them into wide loads detectable by InterleavedAccessPass"


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:73
+
+  /// Scan the function and for interleaved load candidates and execute the
+  /// replacement if applicable.
----------------
"Scan the function for interleaved load candidates" (not "and for")


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:114-117
+/// A and B are the coefficients. Because all computations can be done without
+/// inducing errors. E is the error term and represents some unknown error at
+/// the e most significant bits. Because E is there is no need to store it. The
+/// class only stores the number of error bits 'e'.
----------------
This paragraph doesn't really make sense, particularly the second and fourth sentences. Could you reword it?


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:141
+//
+// The goal is to proof that two loads load consecutive addresses.
+//
----------------
"prove" not "proof"


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:143
+//
+// In this case the the polynomials are constructed by the following
+// steps.
----------------
"the" not "the the"


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:547
+  Polynomial operator-(const Polynomial &o) const {
+    // Return an undefined polynomial incompatible.
+    if (!isCompatibleTo(o))
----------------
"Return an undefined polynomial if incompatible"


================
Comment at: lib/CodeGen/InterleavedLoadCombinePass.cpp:573
+  bool isProvenEqualTo(const Polynomial &o) {
+    // subtract both polynomials and test if it is fully defined and zero.
+    Polynomial r = *this - o;
----------------
Capitalise "Subtract"


https://reviews.llvm.org/D52653





More information about the llvm-commits mailing list