Add/Implement AddresSpace to PseudoSourceValue.

[jsjodin]

Summary

This is built on top of https://reviews.llvm.org/D21688 (not yet committed). Instead of taking the address space as an argument in the constructor we use a the target and callback inside the constructor since the source kind should have enough information for the target to decide which address space to use. This also contains the changes in the rest of the compiler to use the new interface. The TII argument does not have a default value (nullptr) because it should be explicit that the target is not needed and the generic address space will be used in all cases, otherwise it is easy to forget and could cause bugs that would be hard to find.

[arsenm]

lib/CodeGen/PseudoSourceValue.cpp

30 ↗

(On Diff #105530)

If it's required, TII should be a reference or at least this should assert

lib/Target/AMDGPU/SIInstrInfo.cpp

1771–1775 ↗

(On Diff #105530)

These should all be CONSTANT_ADDRESS

1777 ↗

(On Diff #105530)

Prefereable to use the enum value rather than 0

lib/Target/Mips/MipsMachineFunction.cpp

96 ↗

(On Diff #105530)

I thought the PSVManager held a TII reference?

[jsjodin]

lib/CodeGen/PseudoSourceValue.cpp

30 ↗

(On Diff #105530)

Yes, I think it may be better to have an assert. That way we don't have to dereference any pointers at the call sites. My initial idea was that if the target only has a generic address space there would be no need for TII, so a nullptr would be okay, but it would at least be explicit in the call, but that may be more confusing. I will fix this.

lib/Target/Mips/MipsMachineFunction.cpp

96 ↗

(On Diff #105530)

I wasn't sure, but in that case we can use TII, and put an assert in the PSV constructor.

[jsjodin]

Make TII mandatory (assert if given nullptr) in PSV constructor. Add TII as a member to the PSV manager so that it is not needed in the method calls.

[yaxunl]

Ping.

Any progress? D35361 relies on this patch and we need it to fix vload_private/vstore_private failure with new address space mapping.

[hfinkel]

Can you please upload a full-context patch?

[jsjodin]

Merged code with TOT and made full context patch.

[hfinkel]

lib/CodeGen/PseudoSourceValue.cpp

30 ↗

(On Diff #114651)

I don't understand this response. Can you make this a reference? It seems required here in the constructor.

lib/Target/AMDGPU/SIInstrInfo.h

261 ↗

(On Diff #114651)

Line is too long.

[jsjodin]

lib/CodeGen/PseudoSourceValue.cpp

30 ↗

(On Diff #114651)

Yes. it can be made into a reference. I will post a new patch with this.

[jsjodin]

Made changes to address comments by Hal Finkel.

[hfinkel]

LGTM

lib/Target/AMDGPU/SIInstrInfo.cpp

1848 ↗

(On Diff #114868)

Indentation off here.

llvm/trunk/include/llvm/CodeGen/PseudoSourceValue.h

 #include "llvm/IR/ValueMap.h"
 #include <map>
 
 namespace llvm {
 
 class MachineFrameInfo;
 class MachineMemOperand;
 class raw_ostream;
+class TargetInstrInfo;
 
 raw_ostream &operator<<(raw_ostream &OS, const MachineMemOperand &MMO);
 class PseudoSourceValue;
 raw_ostream &operator<<(raw_ostream &OS, const PseudoSourceValue* PSV);
 
 /// Special value supplied for machine level alias analysis. It indicates that
 /// a memory access references the functions stack frame (e.g., a spill slot),
 /// below the stack frame (e.g., argument space), or constant pool.
 class PseudoSourceValue {
 public:
   enum PSVKind {
     Stack,
     GOT,
     JumpTable,
     ConstantPool,
     FixedStack,
     GlobalValueCallEntry,
     ExternalSymbolCallEntry,
     TargetCustom
   };
 
 private:
   PSVKind Kind;
+  unsigned AddressSpace;
   friend raw_ostream &llvm::operator<<(raw_ostream &OS,
                                        const PseudoSourceValue* PSV);
 
   friend class MachineMemOperand; // For printCustom().
 
   /// Implement printing for PseudoSourceValue. This is called from
   /// Value::print or Value's operator<<.
   virtual void printCustom(raw_ostream &O) const;
 
 public:
-  explicit PseudoSourceValue(PSVKind Kind);
+  explicit PseudoSourceValue(PSVKind Kind, const TargetInstrInfo &TII);
 
   virtual ~PseudoSourceValue();
 
   PSVKind kind() const { return Kind; }
 
   bool isStack() const { return Kind == Stack; }
   bool isGOT() const { return Kind == GOT; }
   bool isConstantPool() const { return Kind == ConstantPool; }
   bool isJumpTable() const { return Kind == JumpTable; }
+
+  unsigned getAddressSpace() const { return AddressSpace; }
+
   unsigned getTargetCustom() const {
     return (Kind >= TargetCustom) ? ((Kind+1) - TargetCustom) : 0;
   }
 
   /// Test whether the memory pointed to by this PseudoSourceValue has a
   /// constant value.
   virtual bool isConstant(const MachineFrameInfo *) const;
 
   /// Test whether the memory pointed to by this PseudoSourceValue may also be
   /// pointed to by an LLVM IR Value.
   virtual bool isAliased(const MachineFrameInfo *) const;
 
   /// Return true if the memory pointed to by this PseudoSourceValue can ever
   /// alias an LLVM IR Value.
   virtual bool mayAlias(const MachineFrameInfo *) const;
 };
 
 /// A specialized PseudoSourceValue for holding FixedStack values, which must
 /// include a frame index.
 class FixedStackPseudoSourceValue : public PseudoSourceValue {
   const int FI;
 
 public:
-  explicit FixedStackPseudoSourceValue(int FI)
-      : PseudoSourceValue(FixedStack), FI(FI) {}
+  explicit FixedStackPseudoSourceValue(int FI, const TargetInstrInfo &TII)
+      : PseudoSourceValue(FixedStack, TII), FI(FI) {}
 
   static bool classof(const PseudoSourceValue *V) {
     return V->kind() == FixedStack;
   }
 
   bool isConstant(const MachineFrameInfo *MFI) const override;
 
   bool isAliased(const MachineFrameInfo *MFI) const override;
 
   bool mayAlias(const MachineFrameInfo *) const override;
 
   void printCustom(raw_ostream &OS) const override;
 
   int getFrameIndex() const { return FI; }
 };
 
 class CallEntryPseudoSourceValue : public PseudoSourceValue {
 protected:
-  CallEntryPseudoSourceValue(PSVKind Kind);
+  CallEntryPseudoSourceValue(PSVKind Kind, const TargetInstrInfo &TII);
 
 public:
   bool isConstant(const MachineFrameInfo *) const override;
   bool isAliased(const MachineFrameInfo *) const override;
   bool mayAlias(const MachineFrameInfo *) const override;
 };
 
 /// A specialized pseudo soruce value for holding GlobalValue values.
 class GlobalValuePseudoSourceValue : public CallEntryPseudoSourceValue {
   const GlobalValue *GV;
 
 public:
-  GlobalValuePseudoSourceValue(const GlobalValue *GV);
+  GlobalValuePseudoSourceValue(const GlobalValue *GV,
+                               const TargetInstrInfo &TII);
 
   static bool classof(const PseudoSourceValue *V) {
     return V->kind() == GlobalValueCallEntry;
   }
 
   const GlobalValue *getValue() const { return GV; }
 };
 
 /// A specialized pseudo source value for holding external symbol values.
 class ExternalSymbolPseudoSourceValue : public CallEntryPseudoSourceValue {
   const char *ES;
 
 public:
-  ExternalSymbolPseudoSourceValue(const char *ES);
+  ExternalSymbolPseudoSourceValue(const char *ES, const TargetInstrInfo &TII);
 
   static bool classof(const PseudoSourceValue *V) {
     return V->kind() == ExternalSymbolCallEntry;
   }
 
   const char *getSymbol() const { return ES; }
 };
 
 /// Manages creation of pseudo source values.
 class PseudoSourceValueManager {
+  const TargetInstrInfo &TII;
   const PseudoSourceValue StackPSV, GOTPSV, JumpTablePSV, ConstantPoolPSV;
   std::map<int, std::unique_ptr<FixedStackPseudoSourceValue>> FSValues;
   StringMap<std::unique_ptr<const ExternalSymbolPseudoSourceValue>>
       ExternalCallEntries;
   ValueMap<const GlobalValue *,
            std::unique_ptr<const GlobalValuePseudoSourceValue>>
       GlobalCallEntries;
 
 public:
-  PseudoSourceValueManager();
+  PseudoSourceValueManager(const TargetInstrInfo &TII);
 
   /// Return a pseudo source value referencing the area below the stack frame of
   /// a function, e.g., the argument space.
   const PseudoSourceValue *getStack();
 
   /// Return a pseudo source value referencing the global offset table
   /// (or something the like).
   const PseudoSourceValue *getGOT();

llvm/trunk/include/llvm/Target/TargetInstrInfo.h

 #include "llvm/ADT/None.h"
 #include "llvm/CodeGen/LiveIntervalAnalysis.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineCombinerPattern.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstr.h"
 #include "llvm/CodeGen/MachineLoopInfo.h"
 #include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/CodeGen/PseudoSourceValue.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
 #include <utility>
 #include <vector>
protected:
   virtual bool
   getInsertSubregLikeInputs(const MachineInstr &MI, unsigned DefIdx,
                             RegSubRegPair &BaseReg,
                             RegSubRegPairAndIdx &InsertedReg) const {
     return false;
   }
 
 public:
+  /// getAddressSpaceForPseudoSourceKind - Given the kind of memory
+  /// (e.g. stack) the target returns the corresponding address space.
+  virtual unsigned
+  getAddressSpaceForPseudoSourceKind(PseudoSourceValue::PSVKind Kind) const {
+    return 0;
+  }
+
   /// unfoldMemoryOperand - Separate a single instruction which folded a load or
   /// a store or a load and a store into two or more instruction. If this is
   /// possible, returns true as well as the new instructions by reference.
   virtual bool
   unfoldMemoryOperand(MachineFunction &MF, MachineInstr &MI, unsigned Reg,
                       bool UnfoldLoad, bool UnfoldStore,
                       SmallVectorImpl<MachineInstr *> &NewMIs) const {
     return false;

llvm/trunk/lib/CodeGen/MachineFunction.cpp

if (isFuncletEHPersonality(classifyEHPersonality(
           Fn->hasPersonalityFn() ? Fn->getPersonalityFn() : nullptr))) {
     WinEHInfo = new (Allocator) WinEHFuncInfo();
   }
 
   assert(Target.isCompatibleDataLayout(getDataLayout()) &&
          "Can't create a MachineFunction using a Module with a "
          "Target-incompatible DataLayout attached\n");
 
-  PSVManager = llvm::make_unique<PseudoSourceValueManager>();
+  PSVManager =
+    llvm::make_unique<PseudoSourceValueManager>(*(getSubtarget().
+                                                  getInstrInfo()));
 }
 
 MachineFunction::~MachineFunction() {
   clear();
 }
 
 void MachineFunction::clear() {
   Properties.reset();

llvm/trunk/lib/CodeGen/MachineInstr.cpp

 
 //===----------------------------------------------------------------------===//
 // MachineMemOperand Implementation
 //===----------------------------------------------------------------------===//
 
 /// getAddrSpace - Return the LLVM IR address space number that this pointer
 /// points into.
 unsigned MachinePointerInfo::getAddrSpace() const {
-  if (V.isNull() || V.is<const PseudoSourceValue*>()) return 0;
+  if (V.isNull()) return 0;
+
+  if (V.is<const PseudoSourceValue*>())
+    return V.get<const PseudoSourceValue*>()->getAddressSpace();
+
   return cast<PointerType>(V.get<const Value*>()->getType())->getAddressSpace();
 }
 
 /// isDereferenceable - Return true if V is always dereferenceable for 
 /// Offset + Size byte.
 bool MachinePointerInfo::isDereferenceable(unsigned Size, LLVMContext &C,
                                            const DataLayout &DL) const {
   if (!V.is<const Value*>())

llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp

 //===-- llvm/CodeGen/PseudoSourceValue.cpp ----------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the PseudoSourceValue class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/PseudoSourceValue.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
+#include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 static const char *const PSVNames[] = {
     "Stack", "GOT", "JumpTable", "ConstantPool", "FixedStack",
     "GlobalValueCallEntry", "ExternalSymbolCallEntry"};
 
-PseudoSourceValue::PseudoSourceValue(PSVKind Kind) : Kind(Kind) {}
+PseudoSourceValue::PseudoSourceValue(PSVKind Kind, const TargetInstrInfo &TII)
+    : Kind(Kind) {
+  AddressSpace = TII.getAddressSpaceForPseudoSourceKind(Kind);
+}
+
 
 PseudoSourceValue::~PseudoSourceValue() {}
 
 void PseudoSourceValue::printCustom(raw_ostream &O) const {
   if (Kind < TargetCustom)
     O << PSVNames[Kind];
   else
     O << "TargetCustom" << Kind;
bool FixedStackPseudoSourceValue::mayAlias(const MachineFrameInfo *MFI) const {
   // Spill slots will not alias any LLVM IR value.
   return !MFI->isSpillSlotObjectIndex(FI);
 }
 
 void FixedStackPseudoSourceValue::printCustom(raw_ostream &OS) const {
   OS << "FixedStack" << FI;
 }
 
-CallEntryPseudoSourceValue::CallEntryPseudoSourceValue(PSVKind Kind)
-    : PseudoSourceValue(Kind) {}
+CallEntryPseudoSourceValue::CallEntryPseudoSourceValue(
+    PSVKind Kind, const TargetInstrInfo &TII)
+    : PseudoSourceValue(Kind, TII) {}
 
 bool CallEntryPseudoSourceValue::isConstant(const MachineFrameInfo *) const {
   return false;
 }
 
 bool CallEntryPseudoSourceValue::isAliased(const MachineFrameInfo *) const {
   return false;
 }
 
 bool CallEntryPseudoSourceValue::mayAlias(const MachineFrameInfo *) const {
   return false;
 }
 
 GlobalValuePseudoSourceValue::GlobalValuePseudoSourceValue(
-    const GlobalValue *GV)
-    : CallEntryPseudoSourceValue(GlobalValueCallEntry), GV(GV) {}
-
-ExternalSymbolPseudoSourceValue::ExternalSymbolPseudoSourceValue(const char *ES)
-    : CallEntryPseudoSourceValue(ExternalSymbolCallEntry), ES(ES) {}
-
-PseudoSourceValueManager::PseudoSourceValueManager()
-    : StackPSV(PseudoSourceValue::Stack), GOTPSV(PseudoSourceValue::GOT),
-      JumpTablePSV(PseudoSourceValue::JumpTable),
-      ConstantPoolPSV(PseudoSourceValue::ConstantPool) {}
+    const GlobalValue *GV,
+    const TargetInstrInfo &TII)
+    : CallEntryPseudoSourceValue(GlobalValueCallEntry, TII), GV(GV) {}
+ExternalSymbolPseudoSourceValue::ExternalSymbolPseudoSourceValue(
+    const char *ES, const TargetInstrInfo &TII)
+    : CallEntryPseudoSourceValue(ExternalSymbolCallEntry, TII), ES(ES) {}
+
+PseudoSourceValueManager::PseudoSourceValueManager(
+    const TargetInstrInfo &TIInfo)
+    : TII(TIInfo),
+      StackPSV(PseudoSourceValue::Stack, TII),
+      GOTPSV(PseudoSourceValue::GOT, TII),
+      JumpTablePSV(PseudoSourceValue::JumpTable, TII),
+      ConstantPoolPSV(PseudoSourceValue::ConstantPool, TII) {}
 
 const PseudoSourceValue *PseudoSourceValueManager::getStack() {
   return &StackPSV;
 }
 
 const PseudoSourceValue *PseudoSourceValueManager::getGOT() { return &GOTPSV; }
 
 const PseudoSourceValue *PseudoSourceValueManager::getConstantPool() {
   return &ConstantPoolPSV;
 }
 
 const PseudoSourceValue *PseudoSourceValueManager::getJumpTable() {
   return &JumpTablePSV;
 }
 
-const PseudoSourceValue *PseudoSourceValueManager::getFixedStack(int FI) {
+const PseudoSourceValue *
+PseudoSourceValueManager::getFixedStack(int FI) {
   std::unique_ptr<FixedStackPseudoSourceValue> &V = FSValues[FI];
   if (!V)
-    V = llvm::make_unique<FixedStackPseudoSourceValue>(FI);
+    V = llvm::make_unique<FixedStackPseudoSourceValue>(FI, TII);
   return V.get();
 }
 
 const PseudoSourceValue *
 PseudoSourceValueManager::getGlobalValueCallEntry(const GlobalValue *GV) {
   std::unique_ptr<const GlobalValuePseudoSourceValue> &E =
       GlobalCallEntries[GV];
   if (!E)
-    E = llvm::make_unique<GlobalValuePseudoSourceValue>(GV);
+    E = llvm::make_unique<GlobalValuePseudoSourceValue>(GV, TII);
   return E.get();
 }
 
 const PseudoSourceValue *
 PseudoSourceValueManager::getExternalSymbolCallEntry(const char *ES) {
   std::unique_ptr<const ExternalSymbolPseudoSourceValue> &E =
       ExternalCallEntries[ES];
   if (!E)
-    E = llvm::make_unique<ExternalSymbolPseudoSourceValue>(ES);
+    E = llvm::make_unique<ExternalSymbolPseudoSourceValue>(ES, TII);
   return E.get();
 }

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.h

void insertSelect(MachineBasicBlock &MBB,
                     unsigned DstReg, ArrayRef<MachineOperand> Cond,
                     unsigned TrueReg, unsigned FalseReg) const override;
 
   void insertVectorSelect(MachineBasicBlock &MBB,
                           MachineBasicBlock::iterator I, const DebugLoc &DL,
                           unsigned DstReg, ArrayRef<MachineOperand> Cond,
                           unsigned TrueReg, unsigned FalseReg) const;
 
+  unsigned
+    getAddressSpaceForPseudoSourceKind(PseudoSourceValue::PSVKind Kind) const;
+
   bool
   areMemAccessesTriviallyDisjoint(MachineInstr &MIa, MachineInstr &MIb,
                                   AliasAnalysis *AA = nullptr) const override;
 
   bool isFoldableCopy(const MachineInstr &MI) const;
 
   bool FoldImmediate(MachineInstr &UseMI, MachineInstr &DefMI, unsigned Reg,
                      MachineRegisterInfo *MRI) const final;

llvm/trunk/lib/Target/AMDGPU/SIInstrInfo.cpp

bool SIInstrInfo::isFoldableCopy(const MachineInstr &MI) const {
   case AMDGPU::S_MOV_B64:
   case AMDGPU::COPY:
     return true;
   default:
     return false;
   }
 }
 
+unsigned SIInstrInfo::getAddressSpaceForPseudoSourceKind(
+    PseudoSourceValue::PSVKind Kind) const {
+  switch(Kind) {
+  case PseudoSourceValue::Stack:
+  case PseudoSourceValue::FixedStack:
+    return AMDGPUASI.PRIVATE_ADDRESS;
+  case PseudoSourceValue::ConstantPool:
+  case PseudoSourceValue::GOT:
+  case PseudoSourceValue::JumpTable:
+  case PseudoSourceValue::GlobalValueCallEntry:
+  case PseudoSourceValue::ExternalSymbolCallEntry:
+  case PseudoSourceValue::TargetCustom:
+    return AMDGPUASI.CONSTANT_ADDRESS;
+  default:
+      return AMDGPUASI.FLAT_ADDRESS;
+  }
+}
+
+
 static void removeModOperands(MachineInstr &MI) {
   unsigned Opc = MI.getOpcode();
   int Src0ModIdx = AMDGPU::getNamedOperandIdx(Opc,
                                               AMDGPU::OpName::src0_modifiers);
   int Src1ModIdx = AMDGPU::getNamedOperandIdx(Opc,
                                               AMDGPU::OpName::src1_modifiers);
   int Src2ModIdx = AMDGPU::getNamedOperandIdx(Opc,
                                               AMDGPU::OpName::src2_modifiers);

llvm/trunk/lib/Target/AMDGPU/SIMachineFunctionInfo.h

 #include "AMDGPUMachineFunction.h"
 #include "AMDGPUArgumentUsageInfo.h"
 #include "SIRegisterInfo.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/PseudoSourceValue.h"
+#include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <array>
 #include <cassert>
 #include <utility>
 #include <vector>
 
 namespace llvm {
 
 class MachineFrameInfo;
 class MachineFunction;
 class TargetRegisterClass;
 
 class AMDGPUImagePseudoSourceValue : public PseudoSourceValue {
 public:
-  explicit AMDGPUImagePseudoSourceValue() :
-    PseudoSourceValue(PseudoSourceValue::TargetCustom) {}
+  explicit AMDGPUImagePseudoSourceValue(const TargetInstrInfo &TII) :
+    PseudoSourceValue(PseudoSourceValue::TargetCustom, TII) { }
 
   bool isConstant(const MachineFrameInfo *) const override {
     // This should probably be true for most images, but we will start by being
     // conservative.
     return false;
   }
 
   bool isAliased(const MachineFrameInfo *) const override {
     // FIXME: If we ever change image intrinsics to accept fat pointers, then
     // this could be true for some cases.
     return false;
   }
 
   bool mayAlias(const MachineFrameInfo *) const override {
     // FIXME: If we ever change image intrinsics to accept fat pointers, then
     // this could be true for some cases.
     return false;
   }
 };
 
 class AMDGPUBufferPseudoSourceValue : public PseudoSourceValue {
 public:
-  explicit AMDGPUBufferPseudoSourceValue() :
-    PseudoSourceValue(PseudoSourceValue::TargetCustom) {}
+  explicit AMDGPUBufferPseudoSourceValue(const TargetInstrInfo &TII) :
+    PseudoSourceValue(PseudoSourceValue::TargetCustom, TII) { }
 
   bool isConstant(const MachineFrameInfo *) const override {
     // This should probably be true for most images, but we will start by being
     // conservative.
     return false;
   }
 
   bool isAliased(const MachineFrameInfo *) const override {

llvm/trunk/lib/Target/AMDGPU/SIMachineFunctionInfo.cpp

 #include <vector>
 
 #define MAX_LANES 64
 
 using namespace llvm;
 
 SIMachineFunctionInfo::SIMachineFunctionInfo(const MachineFunction &MF)
   : AMDGPUMachineFunction(MF),
+    BufferPSV(*(MF.getSubtarget().getInstrInfo())),
+    ImagePSV(*(MF.getSubtarget().getInstrInfo())),
     PrivateSegmentBuffer(false),
     DispatchPtr(false),
     QueuePtr(false),
     KernargSegmentPtr(false),
     DispatchID(false),
     FlatScratchInit(false),
     GridWorkgroupCountX(false),
     GridWorkgroupCountY(false),