From 0d47c7aba364962d14e4e25249d75da7bdf29b78 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Lu=C3=ADs=20Marques?= <luismarques@lowrisc.org>
Date: Tue, 5 Nov 2019 09:36:08 +0000
Subject: [PATCH] [RISCV] Add InstrInfo areMemAccessesTriviallyDisjoint hook

Summary: Introduces the `InstrInfo::areMemAccessesTriviallyDisjoint`
hook. The test could check for instruction reorderings, but to avoid
being brittle it just checks instruction dependencies.

Reviewers: asb, lenary
Reviewed By: lenary
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D67046
---
 llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 55 ++++++++++++++++++++++++++++++++
 llvm/lib/Target/RISCV/RISCVInstrInfo.h   |  8 +++++
 llvm/test/CodeGen/RISCV/disjoint.ll      | 26 +++++++++++++++
 3 files changed, 89 insertions(+)
 create mode 100644 llvm/test/CodeGen/RISCV/disjoint.ll

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 0848392..30801e1 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -542,3 +542,58 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
 
   return true;
 }
+
+// Return true if get the base operand, byte offset of an instruction and the
+// memory width. Width is the size of memory that is being loaded/stored.
+bool RISCVInstrInfo::getMemOperandWithOffsetWidth(
+    const MachineInstr &LdSt, const MachineOperand *&BaseReg, int64_t &Offset,
+    unsigned &Width, const TargetRegisterInfo *TRI) const {
+  assert(LdSt.mayLoadOrStore() && "Expected a memory operation.");
+
+  // Here we assume the standard RISC-V ISA, which uses a base+offset
+  // addressing mode. You'll need to relax these conditions to support custom
+  // load/stores instructions.
+  if (LdSt.getNumExplicitOperands() != 3)
+    return false;
+  if (!LdSt.getOperand(1).isReg() || !LdSt.getOperand(2).isImm())
+    return false;
+
+  if (!LdSt.hasOneMemOperand())
+    return false;
+
+  Width = (*LdSt.memoperands_begin())->getSize();
+  BaseReg = &LdSt.getOperand(1);
+  Offset = LdSt.getOperand(2).getImm();
+  return true;
+}
+
+bool RISCVInstrInfo::areMemAccessesTriviallyDisjoint(
+    const MachineInstr &MIa, const MachineInstr &MIb) const {
+  assert(MIa.mayLoadOrStore() && "MIa must be a load or store.");
+  assert(MIb.mayLoadOrStore() && "MIb must be a load or store.");
+
+  if (MIa.hasUnmodeledSideEffects() || MIb.hasUnmodeledSideEffects() ||
+      MIa.hasOrderedMemoryRef() || MIb.hasOrderedMemoryRef())
+    return false;
+
+  // Retrieve the base register, offset from the base register and width. Width
+  // is the size of memory that is being loaded/stored (e.g. 1, 2, 4).  If
+  // base registers are identical, and the offset of a lower memory access +
+  // the width doesn't overlap the offset of a higher memory access,
+  // then the memory accesses are different.
+  const TargetRegisterInfo *TRI = STI.getRegisterInfo();
+  const MachineOperand *BaseOpA = nullptr, *BaseOpB = nullptr;
+  int64_t OffsetA = 0, OffsetB = 0;
+  unsigned int WidthA = 0, WidthB = 0;
+  if (getMemOperandWithOffsetWidth(MIa, BaseOpA, OffsetA, WidthA, TRI) &&
+      getMemOperandWithOffsetWidth(MIb, BaseOpB, OffsetB, WidthB, TRI)) {
+    if (BaseOpA->isIdenticalTo(*BaseOpB)) {
+      int LowOffset = std::min(OffsetA, OffsetB);
+      int HighOffset = std::max(OffsetA, OffsetB);
+      int LowWidth = (LowOffset == OffsetA) ? WidthA : WidthB;
+      if (LowOffset + LowWidth <= HighOffset)
+        return true;
+    }
+  }
+  return false;
+}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index d3ae04a..7a22b9c 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -86,6 +86,14 @@ public:
   bool verifyInstruction(const MachineInstr &MI,
                          StringRef &ErrInfo) const override;
 
+  bool getMemOperandWithOffsetWidth(const MachineInstr &LdSt,
+                                    const MachineOperand *&BaseOp,
+                                    int64_t &Offset, unsigned &Width,
+                                    const TargetRegisterInfo *TRI) const;
+
+  bool areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
+                                       const MachineInstr &MIb) const override;
+
 protected:
   const RISCVSubtarget &STI;
 };
diff --git a/llvm/test/CodeGen/RISCV/disjoint.ll b/llvm/test/CodeGen/RISCV/disjoint.ll
new file mode 100644
index 0000000..5977c61
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/disjoint.ll
@@ -0,0 +1,26 @@
+; REQUIRES: asserts
+; RUN: llc -mtriple=riscv32 -debug-only=machine-scheduler < %s \
+; RUN:   -o /dev/null 2>&1 | FileCheck %s
+; RUN: llc -mtriple=riscv64 -debug-only=machine-scheduler < %s \
+; RUN:   -o /dev/null 2>&1 | FileCheck %s
+
+; This test exercises the areMemAccessesTriviallyDisjoint hook.
+; Test that the two stores are disjoint memory accesses. If the corresponding
+; store machine instructions don't depend on each other, the second store should
+; not appear in the successors list of the first one and the first one should
+; not appear on the predecessors list of the second one.
+define i32 @test_disjoint(i32* %P, i32 %v) {
+entry:
+; CHECK: ********** MI Scheduling **********
+; CHECK-LABEL: test_disjoint:%bb.0
+; CHECK:SU(2):   SW %1:gpr, %0:gpr, 12 :: (store 4 into %ir.arrayidx)
+; CHECK-NOT: Successors:
+; CHECK:SU(3):   SW %1:gpr, %0:gpr, 8 :: (store 4 into %ir.arrayidx1)
+; CHECK: Predecessors:
+; CHECK-NOT:    SU(2): Ord  Latency=0 Memory
+  %arrayidx = getelementptr inbounds i32, i32* %P, i32 3
+  store i32 %v, i32* %arrayidx
+  %arrayidx1 = getelementptr inbounds i32, i32* %P, i32 2
+  store i32 %v, i32* %arrayidx1
+  ret i32 %v
+}
-- 
2.7.4