[AArch64] Enable merging of adjacent zero stores for all subtargets.

[mcrosier]

Summary

This optimization merges adjacent zero stores into a wider store.

e.g.,

strh wzr, [x0]
strh wzr, [x0, #2]
; becomes
str wzr, [x0]

e.g.,

str wzr, [x0]
str wzr, [x0, #4]
; becomes
str xzr, [x0]

Previously, this was only enabled for Kryo and Cortex-A57. I'd like to enable it for all subtargets.

Chad

[MatzeB]

LGTM, nitpick below.

BTW: I tried to find some discussion of why we have this load/store combining feature in the AArch64 target (don't we already have that in GVN and other places in the middleend?) but couldn't find any reviews or discussions that would motivate it.

test/CodeGen/AArch64/arm64-narrow-st-merge.ll

1–8

I don't see the benefits of testing this for every CPU in the AArch64 target. Maybe 1 single RUN: is enough here?

[mcrosier]

In D26396#592260, @MatzeB wrote:

LGTM, nitpick below.

BTW: I tried to find some discussion of why we have this load/store combining feature in the AArch64 target (don't we already have that in GVN and other places in the middleend?) but couldn't find any reviews or discussions that would motivate it.

Thanks for the review, Matthias. I'll follow up on your question with @junbuml as he was the original author of this optimization. I thought we did a similar form of combing at isel lowering, but I may be mistaken.

test/CodeGen/AArch64/arm64-narrow-st-merge.ll

1–8

Sure.

[gberry]

test/CodeGen/AArch64/arm64-narrow-st-merge.ll

9

Perhaps we should add a run that sets FeatureStrictAlign and check that this merging doesn't happen?

[mcrosier]

Committed r286617 to show strict align disables the narrow zero store optimization. Thanks, Geoff.

lib/Target/AArch64/AArch64.td

def FeatureStrictAlign : SubtargetFeature<"strict-align",
                                           "StrictAlign", "true",
                                           "Disallow all unaligned memory "
                                           "access">;
 
 def FeatureReserveX18 : SubtargetFeature<"reserve-x18", "ReserveX18", "true",
                                          "Reserve X18, making it unavailable "
                                          "as a GPR">;
 
-def FeatureMergeNarrowZeroSt : SubtargetFeature<"merge-narrow-zero-st",
-                                                "MergeNarrowZeroStores", "true",
-                                                "Merge narrow zero store "
-                                                "instructions">;
-
 def FeatureUseAA : SubtargetFeature<"use-aa", "UseAA", "true",
                                     "Use alias analysis during codegen">;
 
 def FeatureBalanceFPOps : SubtargetFeature<"balance-fp-ops", "BalanceFPOps",
     "true",
     "balance mix of odd and even D-registers for fp multiply(-accumulate) ops">;
 
 def FeaturePredictableSelectIsExpensive : SubtargetFeature<

 
 def ProcA57     : SubtargetFeature<"a57", "ARMProcFamily", "CortexA57",
                                    "Cortex-A57 ARM processors", [
                                    FeatureBalanceFPOps,
                                    FeatureCRC,
                                    FeatureCrypto,
                                    FeatureCustomCheapAsMoveHandling,
                                    FeatureFPARMv8,
-                                   FeatureMergeNarrowZeroSt,
                                    FeatureNEON,
                                    FeaturePerfMon,
                                    FeaturePostRAScheduler,
                                    FeaturePredictableSelectIsExpensive
                                    ]>;
 
 def ProcA72     : SubtargetFeature<"a72", "ARMProcFamily", "CortexA72",
                                    "Cortex-A72 ARM processors", [
def ProcExynosM2 : SubtargetFeature<"exynosm2", "ARMProcFamily", "ExynosM1",
                                      FeatureZCZeroing]>;
 
 def ProcKryo    : SubtargetFeature<"kryo", "ARMProcFamily", "Kryo",
                                    "Qualcomm Kryo processors", [
                                    FeatureCRC,
                                    FeatureCrypto,
                                    FeatureCustomCheapAsMoveHandling,
                                    FeatureFPARMv8,
-                                   FeatureMergeNarrowZeroSt,
                                    FeatureNEON,
                                    FeaturePerfMon,
                                    FeaturePostRAScheduler,
                                    FeaturePredictableSelectIsExpensive,
                                    FeatureZCZeroing
                                    ]>;
 
 def ProcVulcan  : SubtargetFeature<"vulcan", "ARMProcFamily", "Vulcan",

lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp

bool AArch64LoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
 
   // Resize the modified and used register bitfield trackers.  We do this once
   // per function and then clear the bitfield each time we optimize a load or
   // store.
   ModifiedRegs.resize(TRI->getNumRegs());
   UsedRegs.resize(TRI->getNumRegs());
 
   bool Modified = false;
-  bool enableNarrowZeroStOpt =
-      Subtarget->mergeNarrowStores() && !Subtarget->requiresStrictAlign();
+  bool enableNarrowZeroStOpt = !Subtarget->requiresStrictAlign();
   for (auto &MBB : Fn)
     Modified |= optimizeBlock(MBB, enableNarrowZeroStOpt);
 
   return Modified;
 }
 
 // FIXME: Do we need/want a pre-alloc pass like ARM has to try to keep
 // loads and stores near one another?

lib/Target/AArch64/AArch64Subtarget.h

protected:
   // HasZeroCycleRegMove - Has zero-cycle register mov instructions.
   bool HasZeroCycleRegMove = false;
 
   // HasZeroCycleZeroing - Has zero-cycle zeroing instructions.
   bool HasZeroCycleZeroing = false;
 
   // StrictAlign - Disallow unaligned memory accesses.
   bool StrictAlign = false;
-  bool MergeNarrowZeroStores = false;
   bool UseAA = false;
   bool PredictableSelectIsExpensive = false;
   bool BalanceFPOps = false;
   bool CustomAsCheapAsMove = false;
   bool UsePostRAScheduler = false;
   bool Misaligned128StoreIsSlow = false;
   bool AvoidQuadLdStPairs = false;
   bool UseAlternateSExtLoadCVTF32Pattern = false;
public:
   bool requiresStrictAlign() const { return StrictAlign; }
 
   bool isX18Reserved() const { return ReserveX18; }
   bool hasFPARMv8() const { return HasFPARMv8; }
   bool hasNEON() const { return HasNEON; }
   bool hasCrypto() const { return HasCrypto; }
   bool hasCRC() const { return HasCRC; }
   bool hasRAS() const { return HasRAS; }
-  bool mergeNarrowStores() const { return MergeNarrowZeroStores; }
   bool balanceFPOps() const { return BalanceFPOps; }
   bool predictableSelectIsExpensive() const {
     return PredictableSelectIsExpensive;
   }
   bool hasCustomCheapAsMoveHandling() const { return CustomAsCheapAsMove; }
   bool isMisaligned128StoreSlow() const { return Misaligned128StoreIsSlow; }
   bool avoidQuadLdStPairs() const { return AvoidQuadLdStPairs; }
   bool useAlternateSExtLoadCVTF32Pattern() const {

test/CodeGen/AArch64/arm64-narrow-st-merge.ll

-; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK
-; RUN: llc < %s -mtriple aarch64_be--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK
-; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=kryo -verify-machineinstrs | FileCheck %s --check-prefix=CHECK
+; RUN: llc < %s -mtriple aarch64--none-eabi -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64_be--none-eabi -mcpu=cortex-a57 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=kryo -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=cyclone -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=exynos-m1 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=exynos-m2 -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple aarch64--none-eabi -mcpu=vulcan -verify-machineinstrs | FileCheck %s
 
 ; CHECK-LABEL: Strh_zero
 ; CHECK: str wzr
 define void @Strh_zero(i16* nocapture %P, i32 %n) {
 entry:
   %idxprom = sext i32 %n to i64
   %arrayidx = getelementptr inbounds i16, i16* %P, i64 %idxprom
   store i16 0, i16* %arrayidx
   %add = add nsw i32 %n, 1