From 22dfbc563731c674c7a9e7b4c8292ded823220c5 Mon Sep 17 00:00:00 2001 From: Geoff Berry Date: Fri, 12 Aug 2016 15:26:00 +0000 Subject: [PATCH] [AArch64] Re-factor code shared by AArch64LoadStoreOpt and AArch64InstrInfo. This re-factoring could cause the following slight changes in generated code, though none were observed during testing: - MachineScheduler could decide not to cluster some loads/stores if there are other load/stores with non-pairable opcodes that have the same base register and offset as a pairable set of load/stores. One case of different MachineScheduler pairing did show up in my testing, but it wasn't due to this issue, but due BaseMemOpClusterMutation::clusterNeighboringMemOps() being unstable w.r.t. the order it considers memory operations. See PR28942. - The ImplicitNullChecks optimization could be done for more load/store opcodes. This optimization isn't done for C/C++ code, so it didn't show up in my testing. Reviewers: mcrosier, t.p.northover Subscribers: aemerson, rengolin, mcrosier, llvm-commits Differential Revision: https://reviews.llvm.org/D23365 llvm-svn: 278515 --- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp | 35 +++---------------- llvm/lib/Target/AArch64/AArch64InstrInfo.h | 32 +++++++++++++++++ .../Target/AArch64/AArch64LoadStoreOptimizer.cpp | 40 ++-------------------- 3 files changed, 40 insertions(+), 67 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp index f93288c..45b90a5 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp @@ -1603,36 +1603,8 @@ bool AArch64InstrInfo::isCandidateToMergeOrPair(MachineInstr &MI) const { bool AArch64InstrInfo::getMemOpBaseRegImmOfs( MachineInstr &LdSt, unsigned &BaseReg, int64_t &Offset, const TargetRegisterInfo *TRI) const { - switch (LdSt.getOpcode()) { - default: - return false; - // Scaled instructions. - case AArch64::STRSui: - case AArch64::STRDui: - case AArch64::STRQui: - case AArch64::STRXui: - case AArch64::STRWui: - case AArch64::LDRSui: - case AArch64::LDRDui: - case AArch64::LDRQui: - case AArch64::LDRXui: - case AArch64::LDRWui: - case AArch64::LDRSWui: - // Unscaled instructions. - case AArch64::STURSi: - case AArch64::STURDi: - case AArch64::STURQi: - case AArch64::STURXi: - case AArch64::STURWi: - case AArch64::LDURSi: - case AArch64::LDURDi: - case AArch64::LDURQi: - case AArch64::LDURWi: - case AArch64::LDURXi: - case AArch64::LDURSWi: - unsigned Width; - return getMemOpBaseRegImmOfsWidth(LdSt, BaseReg, Offset, Width, TRI); - }; + unsigned Width; + return getMemOpBaseRegImmOfsWidth(LdSt, BaseReg, Offset, Width, TRI); } bool AArch64InstrInfo::getMemOpBaseRegImmOfsWidth( @@ -1831,6 +1803,9 @@ bool AArch64InstrInfo::shouldClusterMemOps(MachineInstr &FirstLdSt, if (NumLoads > 1) return false; + if (!isPairableLdStInst(FirstLdSt) || !isPairableLdStInst(SecondLdSt)) + return false; + // Can we pair these instructions based on their opcodes? unsigned FirstOpc = FirstLdSt.getOpcode(); unsigned SecondOpc = SecondLdSt.getOpcode(); diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.h b/llvm/lib/Target/AArch64/AArch64InstrInfo.h index d111208..8960b07 100644 --- a/llvm/lib/Target/AArch64/AArch64InstrInfo.h +++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.h @@ -87,6 +87,38 @@ public: /// Return true if this is an unscaled load/store. bool isUnscaledLdSt(MachineInstr &MI) const; + static bool isPairableLdStInst(const MachineInstr &MI) { + switch (MI.getOpcode()) { + default: + return false; + // Scaled instructions. + case AArch64::STRSui: + case AArch64::STRDui: + case AArch64::STRQui: + case AArch64::STRXui: + case AArch64::STRWui: + case AArch64::LDRSui: + case AArch64::LDRDui: + case AArch64::LDRQui: + case AArch64::LDRXui: + case AArch64::LDRWui: + case AArch64::LDRSWui: + // Unscaled instructions. + case AArch64::STURSi: + case AArch64::STURDi: + case AArch64::STURQi: + case AArch64::STURWi: + case AArch64::STURXi: + case AArch64::LDURSi: + case AArch64::LDURDi: + case AArch64::LDURQi: + case AArch64::LDURWi: + case AArch64::LDURXi: + case AArch64::LDURSWi: + return true; + } + } + /// Return true if this is a load/store that can be potentially paired/merged. bool isCandidateToMergeOrPair(MachineInstr &MI) const; diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp index d98ad0c..3b2277b 100644 --- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp +++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp @@ -1744,44 +1744,10 @@ bool AArch64LoadStoreOpt::optimizeBlock(MachineBasicBlock &MBB, // ldp x0, x1, [x2] for (MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end(); MBBI != E;) { - MachineInstr &MI = *MBBI; - switch (MI.getOpcode()) { - default: - // Just move on to the next instruction. - ++MBBI; - break; - // Scaled instructions. - case AArch64::STRSui: - case AArch64::STRDui: - case AArch64::STRQui: - case AArch64::STRXui: - case AArch64::STRWui: - case AArch64::LDRSui: - case AArch64::LDRDui: - case AArch64::LDRQui: - case AArch64::LDRXui: - case AArch64::LDRWui: - case AArch64::LDRSWui: - // Unscaled instructions. - case AArch64::STURSi: - case AArch64::STURDi: - case AArch64::STURQi: - case AArch64::STURWi: - case AArch64::STURXi: - case AArch64::LDURSi: - case AArch64::LDURDi: - case AArch64::LDURQi: - case AArch64::LDURWi: - case AArch64::LDURXi: - case AArch64::LDURSWi: { - if (tryToPairLdStInst(MBBI)) { - Modified = true; - break; - } + if (TII->isPairableLdStInst(*MBBI) && tryToPairLdStInst(MBBI)) + Modified = true; + else ++MBBI; - break; - } - } } // 4) Find base register updates that can be merged into the load or store // as a base-reg writeback. -- 2.7.4