[TII] Allow getMemOpBaseRegImmOfs() to accept negative offsets. NFC.

[mcrosier]

Summary

Offsets can be negative, of course.

This is in preparation for a follow up patch to improve the AArch64 load/store optimization pass. I plan on adding unscaled loads to getMemOpBaseRegImmOfs() so the MI scheduler clusters loads to improve pairing. In AArch64 unscaled loads/stores are used for negatives offsets.

Please have a look.

Chad

[mcrosier]

Adding Matt/Tom as this may impact the R600/SI target.

This change does cause test/CodeGen/AMDGPU/ds_read2st64.ll to fail for the simple_read2st64_f64_max_offset test case. I'll investigate.

[arsenm]

include/llvm/Target/TargetInstrInfo.h

976

I think this should be changed to int64_t to be consistent with other offsets everywhere else

[mcrosier]

Use int64_t, per Matt's request. All lit test are now passing.

[chandlerc]

LGTM

[mcrosier]

Thanks, Chandler. Committed in r263021.

include/llvm/Target/TargetInstrInfo.h

virtual bool shouldScheduleLoadsNear(SDNode *Load1, SDNode *Load2,
                                        int64_t Offset1, int64_t Offset2,
                                        unsigned NumLoads) const {
     return false;
   }
 
   /// Get the base register and byte offset of an instruction that reads/writes
   /// memory.
   virtual bool getMemOpBaseRegImmOfs(MachineInstr *MemOp, unsigned &BaseReg,
-                                     unsigned &Offset,
+                                     int64_t &Offset,
                                      const TargetRegisterInfo *TRI) const {
     return false;
   }
 
   virtual bool enableClusterLoads() const { return false; }
 
   virtual bool shouldClusterLoads(MachineInstr *FirstLdSt,
                                   MachineInstr *SecondLdSt,

lib/CodeGen/ImplicitNullChecks.cpp

 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include "llvm/Target/TargetInstrInfo.h"
 
 using namespace llvm;
 
-static cl::opt<unsigned> PageSize("imp-null-check-page-size",
-                                  cl::desc("The page size of the target in "
-                                           "bytes"),
-                                  cl::init(4096));
+static cl::opt<int> PageSize("imp-null-check-page-size",
+                             cl::desc("The page size of the target in bytes"),
 
 #define DEBUG_TYPE "implicit-null-checks"
 
 STATISTIC(NumImplicitNullChecks,
           "Number of explicit null checks made implicit");
 
 namespace {
 
bool ImplicitNullChecks::analyzeBlockForNullChecks(
 
   unsigned PointerReg = MBP.LHS.getReg();
 
   HazardDetector HD(*TRI);
 
   for (auto MII = NotNullSucc->begin(), MIE = NotNullSucc->end(); MII != MIE;
        ++MII) {
     MachineInstr *MI = &*MII;
-    unsigned BaseReg, Offset;
+    unsigned BaseReg;
+    int64_t Offset;
     if (TII->getMemOpBaseRegImmOfs(MI, BaseReg, Offset, TRI))
       if (MI->mayLoad() && !MI->isPredicable() && BaseReg == PointerReg &&
           Offset < PageSize && MI->getDesc().getNumDefs() <= 1 &&
           HD.isSafeToHoist(MI)) {
         NullCheckList.emplace_back(MI, MBP.ConditionDef, &MBB, NotNullSucc,
                                    NullSucc);
         return true;
       }

lib/CodeGen/MachineScheduler.cpp

 
 namespace {
 /// \brief Post-process the DAG to create cluster edges between neighboring
 /// loads.
 class LoadClusterMutation : public ScheduleDAGMutation {
   struct LoadInfo {
     SUnit *SU;
     unsigned BaseReg;
-    unsigned Offset;
+    int64_t Offset;
     LoadInfo(SUnit *su, unsigned reg, unsigned ofs)
       : SU(su), BaseReg(reg), Offset(ofs) {}
 
     bool operator<(const LoadInfo &RHS) const {
       return std::tie(BaseReg, Offset) < std::tie(RHS.BaseReg, RHS.Offset);
     }
   };
 

 } // anonymous
 
 void LoadClusterMutation::clusterNeighboringLoads(ArrayRef<SUnit*> Loads,
                                                   ScheduleDAGMI *DAG) {
   SmallVector<LoadClusterMutation::LoadInfo,32> LoadRecords;
   for (unsigned Idx = 0, End = Loads.size(); Idx != End; ++Idx) {
     SUnit *SU = Loads[Idx];
     unsigned BaseReg;
-    unsigned Offset;
+    int64_t Offset;
     if (TII->getMemOpBaseRegImmOfs(SU->getInstr(), BaseReg, Offset, TRI))
       LoadRecords.push_back(LoadInfo(SU, BaseReg, Offset));
   }
   if (LoadRecords.size() < 2)
     return;
   std::sort(LoadRecords.begin(), LoadRecords.end());
   unsigned ClusterLength = 1;
   for (unsigned Idx = 0, End = LoadRecords.size(); Idx < (End - 1); ++Idx) {

lib/CodeGen/MachineSink.cpp

static bool SinkingPreventsImplicitNullCheck(MachineInstr *MI,
 
   // Frontends that don't use implicit null checks have no reason to emit
   // branches with make.implicit metadata, and this function should always
   // return false for them.
   if (!PredBB ||
       !PredBB->getTerminator()->getMetadata(LLVMContext::MD_make_implicit))
     return false;
 
-  unsigned BaseReg, Offset;
+  unsigned BaseReg;
+  int64_t Offset;
   if (!TII->getMemOpBaseRegImmOfs(MI, BaseReg, Offset, TRI))
     return false;
 
   if (!(MI->mayLoad() && !MI->isPredicable()))
     return false;
 
   MachineBranchPredicate MBP;
   if (TII->AnalyzeBranchPredicate(*PredMBB, MBP, false))

lib/Target/AArch64/AArch64InstrInfo.h

public:
   /// Return true if pairing the given load or store is hinted to be
   /// unprofitable.
   bool isLdStPairSuppressed(const MachineInstr *MI) const;
 
   /// Hint that pairing the given load or store is unprofitable.
   void suppressLdStPair(MachineInstr *MI) const;
 
   bool getMemOpBaseRegImmOfs(MachineInstr *LdSt, unsigned &BaseReg,
-                             unsigned &Offset,
+                             int64_t &Offset,
                              const TargetRegisterInfo *TRI) const override;
 
   bool getMemOpBaseRegImmOfsWidth(MachineInstr *LdSt, unsigned &BaseReg,
                                   int &Offset, int &Width,
                                   const TargetRegisterInfo *TRI) const;
 
   bool enableClusterLoads() const override { return true; }
 

lib/Target/AArch64/AArch64InstrInfo.cpp

if (MI->memoperands_empty())
     return;
 
   assert(MOSuppressPair < (1 << MachineMemOperand::MOTargetNumBits) &&
          "Too many target MO flags");
   (*MI->memoperands_begin())
       ->setFlags(MOSuppressPair << MachineMemOperand::MOTargetStartBit);
 }
 
-bool
-AArch64InstrInfo::getMemOpBaseRegImmOfs(MachineInstr *LdSt, unsigned &BaseReg,
-                                        unsigned &Offset,
-                                        const TargetRegisterInfo *TRI) const {
+bool AArch64InstrInfo::getMemOpBaseRegImmOfs(
+    MachineInstr *LdSt, unsigned &BaseReg, int64_t &Offset,
   switch (LdSt->getOpcode()) {
   default:
     return false;
   case AArch64::STRSui:
   case AArch64::STRDui:
   case AArch64::STRQui:
   case AArch64::STRXui:
   case AArch64::STRWui:

lib/Target/AArch64/AArch64StorePairSuppress.cpp

bool AArch64StorePairSuppress::runOnMachineFunction(MachineFunction &MF) {
   // computing trace metrics in those cases.
   for (auto &MBB : MF) {
     bool SuppressSTP = false;
     unsigned PrevBaseReg = 0;
     for (auto &MI : MBB) {
       if (!isNarrowFPStore(MI))
         continue;
       unsigned BaseReg;
-      unsigned Offset;
+      int64_t Offset;
       if (TII->getMemOpBaseRegImmOfs(&MI, BaseReg, Offset, TRI)) {
         if (PrevBaseReg == BaseReg) {
           // If this block can take STPs, skip ahead to the next block.
           if (!SuppressSTP && shouldAddSTPToBlock(MI.getParent()))
             break;
           // Otherwise, continue unpairing the stores in this block.
           DEBUG(dbgs() << "Unpairing store " << MI << "\n");
           SuppressSTP = true;

lib/Target/AMDGPU/SIInstrInfo.h

public:
   bool isReallyTriviallyReMaterializable(const MachineInstr *MI,
                                          AliasAnalysis *AA) const override;
 
   bool areLoadsFromSameBasePtr(SDNode *Load1, SDNode *Load2,
                                int64_t &Offset1,
                                int64_t &Offset2) const override;
 
   bool getMemOpBaseRegImmOfs(MachineInstr *LdSt, unsigned &BaseReg,
-                             unsigned &Offset,
+                             int64_t &Offset,
                              const TargetRegisterInfo *TRI) const final;
 
   bool shouldClusterLoads(MachineInstr *FirstLdSt,
                           MachineInstr *SecondLdSt,
                           unsigned NumLoads) const final;
 
   void copyPhysReg(MachineBasicBlock &MBB,
                    MachineBasicBlock::iterator MI, DebugLoc DL,

lib/Target/AMDGPU/SIInstrInfo.cpp

static bool isStride64(unsigned Opc) {
   case AMDGPU::DS_WRITE2ST64_B64:
     return true;
   default:
     return false;
   }
 }
 
 bool SIInstrInfo::getMemOpBaseRegImmOfs(MachineInstr *LdSt, unsigned &BaseReg,
-                                        unsigned &Offset,
+                                        int64_t &Offset,
                                         const TargetRegisterInfo *TRI) const {
   unsigned Opc = LdSt->getOpcode();
 
   if (isDS(*LdSt)) {
     const MachineOperand *OffsetImm = getNamedOperand(*LdSt,
                                                       AMDGPU::OpName::offset);
     if (OffsetImm) {
       // Normal, single offset LDS instruction.
static bool offsetsDoNotOverlap(int WidthA, int OffsetA,
   int LowOffset = OffsetA < OffsetB ? OffsetA : OffsetB;
   int HighOffset = OffsetA < OffsetB ? OffsetB : OffsetA;
   int LowWidth = (LowOffset == OffsetA) ? WidthA : WidthB;
   return LowOffset + LowWidth <= HighOffset;
 }
 
 bool SIInstrInfo::checkInstOffsetsDoNotOverlap(MachineInstr *MIa,
                                                MachineInstr *MIb) const {
-  unsigned BaseReg0, Offset0;
-  unsigned BaseReg1, Offset1;
+  unsigned BaseReg0, BaseReg1;
+  int64_t Offset0, Offset1;
 
   if (getMemOpBaseRegImmOfs(MIa, BaseReg0, Offset0, &RI) &&
       getMemOpBaseRegImmOfs(MIb, BaseReg1, Offset1, &RI)) {
     assert(MIa->hasOneMemOperand() && MIb->hasOneMemOperand() &&
            "read2 / write2 not expected here yet");
     unsigned Width0 = (*MIa->memoperands_begin())->getSize();
     unsigned Width1 = (*MIb->memoperands_begin())->getSize();
     if (BaseReg0 == BaseReg1 &&

lib/Target/AMDGPU/SIMachineScheduler.cpp

void SIScheduleDAGMI::schedule()
   IsHighLatencySU.clear();
 
   IsLowLatencySU.resize(SUnits.size(), 0);
   LowLatencyOffset.resize(SUnits.size(), 0);
   IsHighLatencySU.resize(SUnits.size(), 0);
 
   for (unsigned i = 0, e = (unsigned)SUnits.size(); i != e; ++i) {
     SUnit *SU = &SUnits[i];
-    unsigned BaseLatReg, OffLatReg;
+    unsigned BaseLatReg;
+    int64_t OffLatReg;
     if (SITII->isLowLatencyInstruction(SU->getInstr())) {
       IsLowLatencySU[i] = 1;
       if (SITII->getMemOpBaseRegImmOfs(SU->getInstr(), BaseLatReg,
                                       OffLatReg, TRI))
         LowLatencyOffset[i] = OffLatReg;
     } else if (SITII->isHighLatencyInstruction(SU->getInstr()))
       IsHighLatencySU[i] = 1;
   }

lib/Target/X86/X86InstrInfo.h

public:
   // Branch analysis.
   bool isUnpredicatedTerminator(const MachineInstr &MI) const override;
   bool AnalyzeBranch(MachineBasicBlock &MBB, MachineBasicBlock *&TBB,
                      MachineBasicBlock *&FBB,
                      SmallVectorImpl<MachineOperand> &Cond,
                      bool AllowModify) const override;
 
   bool getMemOpBaseRegImmOfs(MachineInstr *LdSt, unsigned &BaseReg,
-                             unsigned &Offset,
+                             int64_t &Offset,
                              const TargetRegisterInfo *TRI) const override;
   bool AnalyzeBranchPredicate(MachineBasicBlock &MBB,
                               TargetInstrInfo::MachineBranchPredicate &MBP,
                               bool AllowModify = false) const override;
 
   unsigned RemoveBranch(MachineBasicBlock &MBB) const override;
   unsigned InsertBranch(MachineBasicBlock &MBB, MachineBasicBlock *TBB,
                         MachineBasicBlock *FBB, ArrayRef<MachineOperand> Cond,

lib/Target/X86/X86InstrInfo.cpp

case 64:
     if (isStackAligned)
       return load ? X86::VMOVAPSZrm : X86::VMOVAPSZmr;
     else
       return load ? X86::VMOVUPSZrm : X86::VMOVUPSZmr;
   }
 }
 
 bool X86InstrInfo::getMemOpBaseRegImmOfs(MachineInstr *MemOp, unsigned &BaseReg,
-                                         unsigned &Offset,
+                                         int64_t &Offset,
                                          const TargetRegisterInfo *TRI) const {
   const MCInstrDesc &Desc = MemOp->getDesc();
   int MemRefBegin = X86II::getMemoryOperandNo(Desc.TSFlags, MemOp->getOpcode());
   if (MemRefBegin < 0)
     return false;
 
   MemRefBegin += X86II::getOperandBias(Desc);