[PATCH] D20250: [ARM/AArch64] Match additional patterns to ldN instructions

[Tim Northover via llvm-commits]

t.p.northover added a subscriber: t.p.northover.
t.p.northover added a comment.

I think the testing should probably be overhauled. A couple of CodeGen ones are OK, but this is an IR-level pass that should be tested by opt really.

The coverage is also pretty weak: it's basically just one test where the optimization applies. What if the shuffle doesn't dominate the extract? Or there's no suitable shuffle? What if there's more than one extract? What if the extract index is undef or variable?

Other than that, it looks reasonable to me. Couple of minor comments.

Tim.


================
Comment at: lib/CodeGen/InterleavedAccessPass.cpp:249
@@ +248,3 @@
+  // use the shufflevector instructions instead of the load.
+  if (!canReplaceExtracts(Extracts, Shuffles))
+    return false;
----------------
Perhaps "tryReplaceExtracts" since the function actually makes changes.

================
Comment at: lib/CodeGen/InterleavedAccessPass.cpp:322-325
@@ +321,6 @@
+    auto *Vector = Replacement.second.first;
+    auto *Index = ConstantInt::get(Type::getInt32Ty(Vector->getContext()),
+                                   Replacement.second.second);
+    Extract->replaceAllUsesWith(
+        ExtractElementInst::Create(Vector, Index, "", Extract));
+    Extract->eraseFromParent();
----------------
This should probably use IRBuilder (to get the debug location transferred if nothing else).


http://reviews.llvm.org/D20250