[PSV] Make the API for TargetCustom values not trigger UB

[kariddi]

Summary

Hi,

this patch tries to fix a problem with the API for TargetCustom PseudoSourceValue types that is currently exhibiting UB if you try to use it in the intended way with more than 1 PseudoSourceValue custom kind.

How the API is intended to be used (by looking at getTargetCustom() or the printCustom() method) is that passing a value >= PSVKind::TargetCustom as the "Kind" parameter of the PseudoSourceValue constructor is a way to specify that our PSV is of a custom kind. So, if you have 2 different custom PSV types you should pass as the Kind parameter for the first one PSVKind::TargetCustom and for the second on "PSVKind::TargetCustom+1"

getTargetCustom() will return 0 if the PSV is not custom, 1 if it is the first custom PSV, 2 if it is the second

The problem is that PSVKind::TargetCustom+1 is not an entry of the PSVKind enum and passing that as a value to the constructor is theoretically undefined behavior and is reported as such by the UB sanitizer.

This results in the fact that having more than 1 PSV custom kind is actually not possible without triggering UB.

This patch tries to address it to use "unsigned" (the underlying value of the enum) to store and pass around the Kind instead of the PSVKind enum type.

This patch is NFC otherwise

[arsenm]

LGTM

[kariddi]

Thanks Matt, committed r340200

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

 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 {
+  enum PSVKind : unsigned {
     Stack,
     GOT,
     JumpTable,
     ConstantPool,
     FixedStack,
     GlobalValueCallEntry,
     ExternalSymbolCallEntry,
     TargetCustom
   };
 
 private:
-  PSVKind Kind;
+  unsigned 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, const TargetInstrInfo &TII);
+  explicit PseudoSourceValue(unsigned Kind, const TargetInstrInfo &TII);
 
   virtual ~PseudoSourceValue();
 
-  PSVKind kind() const { return Kind; }
+  unsigned 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; }
 
public:
 
   void printCustom(raw_ostream &OS) const override;
 
   int getFrameIndex() const { return FI; }
 };
 
 class CallEntryPseudoSourceValue : public PseudoSourceValue {
 protected:
-  CallEntryPseudoSourceValue(PSVKind Kind, const TargetInstrInfo &TII);
+  CallEntryPseudoSourceValue(unsigned 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.

llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h

getInsertSubregLikeInputs(const MachineInstr &MI, unsigned DefIdx,
                             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 {
+  getAddressSpaceForPseudoSourceKind(unsigned 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,

llvm/trunk/lib/CodeGen/PseudoSourceValue.cpp

 #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, const TargetInstrInfo &TII)
+PseudoSourceValue::PseudoSourceValue(unsigned Kind, const TargetInstrInfo &TII)
     : Kind(Kind) {
   AddressSpace = TII.getAddressSpaceForPseudoSourceKind(Kind);
 }
 
 
 PseudoSourceValue::~PseudoSourceValue() {}
 
 void PseudoSourceValue::printCustom(raw_ostream &O) const {
bool FixedStackPseudoSourceValue::mayAlias(const MachineFrameInfo *MFI) const {
   return !MFI->isSpillSlotObjectIndex(FI);
 }
 
 void FixedStackPseudoSourceValue::printCustom(raw_ostream &OS) const {
   OS << "FixedStack" << FI;
 }
 
 CallEntryPseudoSourceValue::CallEntryPseudoSourceValue(
-    PSVKind Kind, const TargetInstrInfo &TII)
+    unsigned Kind, const TargetInstrInfo &TII)
     : PseudoSourceValue(Kind, TII) {}
 
 bool CallEntryPseudoSourceValue::isConstant(const MachineFrameInfo *) const {
   return false;
 }
 
 bool CallEntryPseudoSourceValue::isAliased(const MachineFrameInfo *) const {
   return false;

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

bool isRegisterStore(const MachineInstr &MI) const {
     return get(MI.getOpcode()).TSFlags & R600InstrFlags::REGISTER_STORE;
   }
 
   bool isRegisterLoad(const MachineInstr &MI) const {
     return get(MI.getOpcode()).TSFlags & R600InstrFlags::REGISTER_LOAD;
   }
 
   unsigned getAddressSpaceForPseudoSourceKind(
-      PseudoSourceValue::PSVKind Kind) const override;
+      unsigned Kind) const override;
 };
 
 namespace R600 {
 
 int getLDSNoRetOp(uint16_t Opcode);
 
 } //End namespace AMDGPU
 
 } // End llvm namespace
 
 #endif

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

if (HAS_NATIVE_OPERANDS(TargetFlags)) {
     MachineOperand &FlagOp = getFlagOp(MI);
     unsigned InstFlags = FlagOp.getImm();
     InstFlags &= ~(Flag << (NUM_MO_FLAGS * Operand));
     FlagOp.setImm(InstFlags);
   }
 }
 
 unsigned R600InstrInfo::getAddressSpaceForPseudoSourceKind(
-    PseudoSourceValue::PSVKind Kind) const {
+    unsigned Kind) const {
   switch (Kind) {
   case PseudoSourceValue::Stack:
   case PseudoSourceValue::FixedStack:
     return ST.getAMDGPUAS().PRIVATE_ADDRESS;
   case PseudoSourceValue::ConstantPool:
   case PseudoSourceValue::GOT:
   case PseudoSourceValue::JumpTable:
   case PseudoSourceValue::GlobalValueCallEntry:
   case PseudoSourceValue::ExternalSymbolCallEntry:
   case PseudoSourceValue::TargetCustom:
     return ST.getAMDGPUAS().CONSTANT_ADDRESS;
   }
   llvm_unreachable("Invalid pseudo source kind");
   return ST.getAMDGPUAS().PRIVATE_ADDRESS;
 }

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

void insertSelect(MachineBasicBlock &MBB,
                     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 override;
+             unsigned Kind) const override;
 
   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,

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

bool SIInstrInfo::isFoldableCopy(const MachineInstr &MI) const {
   case AMDGPU::COPY:
     return true;
   default:
     return false;
   }
 }
 
 unsigned SIInstrInfo::getAddressSpaceForPseudoSourceKind(
-    PseudoSourceValue::PSVKind Kind) const {
+    unsigned Kind) const {
   switch(Kind) {
   case PseudoSourceValue::Stack:
   case PseudoSourceValue::FixedStack:
     return ST.getAMDGPUAS().PRIVATE_ADDRESS;
   case PseudoSourceValue::ConstantPool:
   case PseudoSourceValue::GOT:
   case PseudoSourceValue::JumpTable:
   case PseudoSourceValue::GlobalValueCallEntry: