[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 11:42:16 PST 2015


t.p.northover added inline comments.

================
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);
----------------
junbuml wrote:
> t.p.northover wrote:
> > That hard-coded 2 looks very suspicious to me. Won't permit an incorrect STRHHui -> STRWui transformation?
> Since we merge two narrow zero stores to one zero store by doubling the datasize, I think 2 is just right value here for scaled case. For unscaled case, we don't need to have this check. 
> 
> Since this patch transforms narrow stores which store only zero, I think STRHHui -> STRWui is also valid. Please let me know if there is any case I missed about the incorrect STRHHui -> STRWui transformation.
> 
STRHHui -> STRWui is fine, but I think (can't test, the patch no longer applies cleanly to trunk) the following would go wrong:

    strh wzr, [x0, #2]
    strh wzr, [x0, #4]

In this case MinOffset == 2, alignTo(MinOffset, 2) == 2, but "str wzr [x0, #2]" isn't encodable.


http://reviews.llvm.org/D14514





More information about the llvm-commits mailing list