[PATCH] D14514: [AArch64]Merge narrow zero stores to wider single store

Tim Northover via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 10 09:53:09 PST 2015


t.p.northover added inline comments.

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:706-707
@@ +705,4 @@
+    // Change the scaled offset from small to large type.
+    if (!IsUnscaled)
+      OffsetImm /= 2;
+    MIB = BuildMI(*I->getParent(), InsertionPoint, I->getDebugLoc(),
----------------
junbuml wrote:
> t.p.northover wrote:
> > What happens here for sequences like
> > 
> >     strb wzr, [x0, #1]
> >     strb wzr, [x0, #2]
> > 
> > ? It looks like you might try to produce "strh wzr, [x0, #1]" (using STRHHui) which is invalid. This also applies to the code recently added around line 537.
> This pass will not merge the input you mentioned above. In findMatchingInsn(), we check if the new scaled wide load can express the scaled narrow inputs in around line 948 below in this patch.
>  
> 
Ah, I see. Could we put some divisibility asserts in here just in case future modifications mess things around?

================
Comment at: lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:952
@@ -903,3 +951,3 @@
           // input, bail and keep looking.
           if (!IsUnscaled && alignTo(MinOffset, 2) != MinOffset) {
             trackRegDefsUses(MI, ModifiedRegs, UsedRegs, TRI);
----------------
That hard-coded 2 looks very suspicious to me. Won't permit an incorrect STRHHui -> STRWui transformation?


http://reviews.llvm.org/D14514





More information about the llvm-commits mailing list