[PATCH] D18890: [AArch64] add SSA Load Store optimization pass

[Jongwon Lee via llvm-commits]

JongwonLee marked 4 inline comments as done.
JongwonLee added a comment.

In http://reviews.llvm.org/D18890#410557, @jmolloy wrote:

> Hi,
>
> I'm confused about why you're doing this optimization here instead of much earlier in the compiler. This is basically memcpy idiom recognition - taking multiple 32-bit loads/stores and converting them into a llvm.memcpy intrinsic for perfect lowering should do the same job, and would work for all backends.
>
> Have you investigated doing this much earlier (IR, pre-ISel?)
>
> James


Hi James.
This patch is considering only AArch64 not other backends. Actually, earlier level work that can merge 32-bit loads/stores is IR-level SLP vectorization. Current SLP vectorization does not support 64-bit width packing, so I tried to extend the range of SLP vectorization. Please see the patch (http://reviews.llvm.org/D18237, http://reviews.llvm.org/D19151).


================
Comment at: lib/Target/AArch64/AArch64SSALoadStoreOptimizer.cpp:117
@@ +116,3 @@
+// the instruction located later between MIa and MIb.
+bool AArch64SSALoadStoreOpt::hasAliasBetween(MachineInstr *MIa,
+                                             MachineInstr *MIb) {
----------------
junbuml wrote:
> This function basically assume that MIa and MIb is in the same basic block. But, isn't it possible that  MIa and MIb could be in different basic blocks?  
It would be possible that the load/store instructions in different basic blocks, but it's not considered in this patch.

================
Comment at: lib/Target/AArch64/AArch64SSALoadStoreOptimizer.cpp:144
@@ +143,3 @@
+      return false;
+    } else if (MI->mayStore()) {
+      // Check if MI is aliased with EndMI.
----------------
junbuml wrote:
> When you check alias between two stores you should also check if there is any  load aliased with the second store if you always move the second to the first.
Thanks. I will fix this.


http://reviews.llvm.org/D18890