[Xtensa] Codegen support for memory operations

[andreisfr]

Summary

Implement codegen for address operands of the load operations
L8UI/L16UI/L16SI/L32I and store operations S8I/S16I/S32I.

[andreisfr]

Minor code formatting.

[myhsu]

I just realized that this patch along with nearly all other patches in the series don't have tests, what's your plan on that?
Also, it might be a good idea to add more reviewers, potentially backend maintainers.

[andreisfr]

Hi @myhsu, thank you for your comment. We have plans to add also some tests to the current functionality.

[aykevl]

@andreisfr I believe every patch should stand on its own and have tests for its own functionality. Even if the backend won't be fully functional without all parts included, each commit should still have the ability to be tested as it is added.

[andreisfr]

Added minimal support for stack operations like storeRegToStackSlot/loadRegFromStackSlot/eliminateFrameIndex functions to make possible implement different tests. Added test for load/store operations on stack.

[arsenm]

Basically lgtm with some nits

llvm/lib/Target/Xtensa/XtensaISelDAGToDAG.cpp

55

s/is not/are not/

Can you make this a DiagnosticInfoUnsupported instead?

67–81

Move this to a separate util function

llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp

67–68

define and init on same line?

80

init on definition

86–100

Move to separate util function

[arsenm]

llvm/test/CodeGen/Xtensa/stack-access.ll

21–22

weird indent

[andreisfr]

Updated according latest changes in main branch and reviewers comments.

llvm/lib/Target/Xtensa/XtensaISelDAGToDAG.cpp

public:
   StringRef getPassName() const override {
     return "Xtensa DAG->DAG Pattern Instruction Selection";
   }
 
   void Select(SDNode *Node) override;
 
   bool selectMemRegAddr(SDValue Addr, SDValue &Base, SDValue &Offset,
                         int Scale) {
-    report_fatal_error("MemReg address is not implemented yet");
+    EVT ValTy = Addr.getValueType();
+
+    // if Address is FI, get the TargetFrameIndex.
+    if (FrameIndexSDNode *FIN = dyn_cast<FrameIndexSDNode>(Addr)) {
+      Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), ValTy);
+      Offset = CurDAG->getTargetConstant(0, SDLoc(Addr), ValTy);
+
+      return true;
+    }
+
+    if (TM.isPositionIndependent())
+      report_fatal_error("PIC relocations is not supported");
+
+    if ((Addr.getOpcode() == ISD::TargetExternalSymbol ||
+         Addr.getOpcode() == ISD::TargetGlobalAddress))
+      return false;
+
+    // Addresses of the form FI+const or FI|const
+    bool Valid = false;
+    if (CurDAG->isBaseWithConstantOffset(Addr)) {
+      ConstantSDNode *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1));
+      int64_t OffsetVal = CN->getSExtValue();
+
+      switch (Scale) {
+      case 1:
+        Valid = (OffsetVal >= 0 && OffsetVal <= 255);
+        break;
+      case 2:
+        Valid =
+            (OffsetVal >= 0 && OffsetVal <= 510) && ((OffsetVal & 0x1) == 0);
+        break;
+      case 4:
+        Valid =
+            (OffsetVal >= 0 && OffsetVal <= 1020) && ((OffsetVal & 0x3) == 0);
+        break;
+      default:
+        break;
+      }
+
+      if (Valid) {
+        // If the first operand is a FI, get the TargetFI Node
+        if (FrameIndexSDNode *FIN =
+                dyn_cast<FrameIndexSDNode>(Addr.getOperand(0)))
+          Base = CurDAG->getTargetFrameIndex(FIN->getIndex(), ValTy);
+        else
+          Base = Addr.getOperand(0);
+
+        Offset =
+            CurDAG->getTargetConstant(CN->getZExtValue(), SDLoc(Addr), ValTy);
+        return true;
+      }
+    }
+
+    // Last case
+    Base = Addr;
+    Offset = CurDAG->getTargetConstant(0, SDLoc(Addr), Addr.getValueType());
     return true;
   }
 
   bool selectMemRegAddrISH1(SDValue Addr, SDValue &Base, SDValue &Offset) {
     return selectMemRegAddr(Addr, Base, Offset, 1);
   }
 
   bool selectMemRegAddrISH2(SDValue Addr, SDValue &Base, SDValue &Offset) {

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

XtensaTargetLowering::XtensaTargetLowering(const TargetMachine &tm,
 
   setSchedulingPreference(Sched::RegPressure);
 
   setBooleanContents(ZeroOrOneBooleanContent);
   setBooleanVectorContents(ZeroOrOneBooleanContent);
 
   setMinFunctionAlignment(Align(4));
 
+  // No sign extend instructions for i1
+  for (MVT VT : MVT::integer_valuetypes()) {
+    setLoadExtAction(ISD::SEXTLOAD, VT, MVT::i1, Promote);
+    setLoadExtAction(ISD::ZEXTLOAD, VT, MVT::i1, Promote);
+    setLoadExtAction(ISD::EXTLOAD, VT, MVT::i1, Promote);
+  }
+
   // Compute derived properties from the register classes
   computeRegisterProperties(STI.getRegisterInfo());
 }
 
 //===----------------------------------------------------------------------===//
 // Calling conventions
 //===----------------------------------------------------------------------===//
 

llvm/lib/Target/Xtensa/XtensaInstrInfo.h

class XtensaInstrInfo : public XtensaGenInstrInfo {
   const XtensaRegisterInfo RI;
   XtensaSubtarget &STI;
 
 public:
   XtensaInstrInfo(XtensaSubtarget &STI);
 
   // Return the XtensaRegisterInfo, which this class owns.
   const XtensaRegisterInfo &getRegisterInfo() const { return RI; }
+
+  void copyPhysReg(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
+                   const DebugLoc &DL, MCRegister DestReg, MCRegister SrcReg,
+                   bool KillSrc) const override;
+  void storeRegToStackSlot(MachineBasicBlock &MBB,
+                           MachineBasicBlock::iterator MBBI, Register SrcReg,
+                           bool isKill, int FrameIndex,
+                           const TargetRegisterClass *RC,
+                           const TargetRegisterInfo *TRI,
+                           Register VReg) const override;
+  void loadRegFromStackSlot(MachineBasicBlock &MBB,
+                            MachineBasicBlock::iterator MBBI, Register DestReg,
+                            int FrameIdx, const TargetRegisterClass *RC,
+                            const TargetRegisterInfo *TRI,
+                            Register VReg) const override;
+
+  // Get the load and store opcodes for a given register class and offset.
+  void getLoadStoreOpcodes(const TargetRegisterClass *RC, unsigned &LoadOpcode,
+                           unsigned &StoreOpcode, int64_t offset) const;
 };
 } // end namespace llvm
 
 #endif /* LLVM_LIB_TARGET_XTENSA_XTENSAINSTRINFO_H */

llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp

 //
 // This file contains the Xtensa implementation of the TargetInstrInfo class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "XtensaInstrInfo.h"
 #include "XtensaTargetMachine.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 
 #define GET_INSTRINFO_CTOR_DTOR
 #include "XtensaGenInstrInfo.inc"
 
 using namespace llvm;
 
+static inline const MachineInstrBuilder &
+addFrameReference(const MachineInstrBuilder &MIB, int FI) {
+  MachineInstr *MI = MIB;
+  MachineFunction &MF = *MI->getParent()->getParent();
+  MachineFrameInfo &MFFrame = MF.getFrameInfo();
+  const MCInstrDesc &MCID = MI->getDesc();
+  MachineMemOperand::Flags Flags = MachineMemOperand::MONone;
+  if (MCID.mayLoad())
+    Flags |= MachineMemOperand::MOLoad;
+  if (MCID.mayStore())
+    Flags |= MachineMemOperand::MOStore;
+  int64_t Offset = 0;
+  Align Alignment = MFFrame.getObjectAlign(FI);
+
+  MachineMemOperand *MMO =
+      MF.getMachineMemOperand(MachinePointerInfo::getFixedStack(MF, FI, Offset),
+                              Flags, MFFrame.getObjectSize(FI), Alignment);
+  return MIB.addFrameIndex(FI).addImm(Offset).addMemOperand(MMO);
+}
+
 XtensaInstrInfo::XtensaInstrInfo(XtensaSubtarget &sti)
     : XtensaGenInstrInfo(), RI(sti), STI(sti) {}
+
+void XtensaInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
+                                  MachineBasicBlock::iterator MBBI,
+                                  const DebugLoc &DL, MCRegister DestReg,
+                                  MCRegister SrcReg, bool KillSrc) const {
+  // when we are copying a phys reg we want the bits for fp
+  if (Xtensa::ARRegClass.contains(DestReg, SrcReg))
+    BuildMI(MBB, MBBI, DL, get(Xtensa::OR), DestReg)
+        .addReg(SrcReg, getKillRegState(KillSrc))
+        .addReg(SrcReg, getKillRegState(KillSrc));
+  else
+    llvm_unreachable("Impossible reg-to-reg copy");
+}
+
+void XtensaInstrInfo::storeRegToStackSlot(
+    MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI, Register SrcReg,
+    bool isKill, int FrameIdx, const TargetRegisterClass *RC,
+    const TargetRegisterInfo *TRI, Register VReg) const {
+  DebugLoc DL = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
+  unsigned LoadOpcode, StoreOpcode;
+  getLoadStoreOpcodes(RC, LoadOpcode, StoreOpcode, FrameIdx);
+  addFrameReference(BuildMI(MBB, MBBI, DL, get(StoreOpcode))
+                        .addReg(SrcReg, getKillRegState(isKill)),
+                    FrameIdx);
+}
+
+void XtensaInstrInfo::loadRegFromStackSlot(MachineBasicBlock &MBB,
+                                           MachineBasicBlock::iterator MBBI,
+                                           Register DestReg, int FrameIdx,
+                                           const TargetRegisterClass *RC,
+                                           const TargetRegisterInfo *TRI,
+                                           Register VReg) const {
+  DebugLoc DL = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
+  unsigned LoadOpcode, StoreOpcode;
+  getLoadStoreOpcodes(RC, LoadOpcode, StoreOpcode, FrameIdx);
+  addFrameReference(BuildMI(MBB, MBBI, DL, get(LoadOpcode), DestReg), FrameIdx);
+}
+
+void XtensaInstrInfo::getLoadStoreOpcodes(const TargetRegisterClass *RC,
+                                          unsigned &LoadOpcode,
+                                          unsigned &StoreOpcode,
+                                          int64_t offset) const {
+  if (RC == &Xtensa::ARRegClass) {
+    LoadOpcode = Xtensa::L32I;
+    StoreOpcode = Xtensa::S32I;
+  } else
+    llvm_unreachable("Unsupported regclass to load or store");
+}

llvm/lib/Target/Xtensa/XtensaRegisterInfo.h

public:
   getCalleeSavedRegs(const MachineFunction *MF = 0) const override;
   const uint32_t *getCallPreservedMask(const MachineFunction &MF,
                                        CallingConv::ID) const override;
   BitVector getReservedRegs(const MachineFunction &MF) const override;
   bool eliminateFrameIndex(MachineBasicBlock::iterator MI, int SPAdj,
                            unsigned FIOperandNum,
                            RegScavenger *RS = nullptr) const override;
   Register getFrameRegister(const MachineFunction &MF) const override;
+
+private:
+  bool eliminateFI(MachineBasicBlock::iterator II, unsigned OpNo,
+                   int FrameIndex, uint64_t StackSize, int64_t SPOffset) const;
 };
 
 } // end namespace llvm
 
 #endif /* LLVM_LIB_TARGET_XTENSA_REGISTERINFO_H */

llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp

 //
 // This file contains the Xtensa implementation of the TargetRegisterInfo class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "XtensaRegisterInfo.h"
 #include "XtensaInstrInfo.h"
 #include "XtensaSubtarget.h"
+#include "llvm/CodeGen/MachineFrameInfo.h"
+#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 #define DEBUG_TYPE "xtensa-reg-info"
 
if (TFI->hasFP(MF)) {
     Reserved.set(getFrameRegister(MF));
   }
 
   // Reserve stack pointer.
   Reserved.set(Xtensa::SP);
   return Reserved;
 }
 
+bool XtensaRegisterInfo::eliminateFI(MachineBasicBlock::iterator II,
+                                     unsigned OpNo, int FrameIndex,
+                                     uint64_t StackSize,
+                                     int64_t SPOffset) const {
+  MachineInstr &MI = *II;
+  unsigned FrameReg;
+  FrameReg = Xtensa::SP;
+
+  // Calculate final offset.
+  // - There is no need to change the offset if the frame object is one of the
+  //   following: an outgoing argument, pointer to a dynamically allocated
+  //   stack space or a $gp restore location,
+  // - If the frame object is any of the following, its offset must be adjusted
+  //   by adding the size of the stack:
+  //   incoming argument, callee-saved register location or local variable.
+  bool IsKill = false;
+  int64_t Offset;
+
+  Offset = SPOffset + (int64_t)StackSize;
+  Offset += MI.getOperand(OpNo + 1).getImm();
+
+  LLVM_DEBUG(errs() << "Offset     : " << Offset << "\n"
+                    << "<--------->\n");
+
+  bool Valid = false;
+  switch (MI.getOpcode()) {
+  case Xtensa::L8UI:
+  case Xtensa::S8I:
+    Valid = (Offset >= 0 && Offset <= 255);
+    break;
+  case Xtensa::L16SI:
+  case Xtensa::L16UI:
+  case Xtensa::S16I:
+    Valid = (Offset >= 0 && Offset <= 510);
+    break;
+  default:
+    Valid = (Offset >= 0 && Offset <= 1020);
+    break;
+  }
+
+  // If MI is not a debug value, make sure Offset fits in the 16-bit immediate
+  // field.
+  if (!MI.isDebugValue() && !Valid) {
+    report_fatal_error("Load immediate not supported yet");
+  }
+
+  MI.getOperand(OpNo).ChangeToRegister(FrameReg, false, false, IsKill);
+  MI.getOperand(OpNo + 1).ChangeToImmediate(Offset);
+
+  return false;
+}
+
 bool XtensaRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
                                              int SPAdj, unsigned FIOperandNum,
                                              RegScavenger *RS) const {
-  report_fatal_error("Eliminate frame index not supported yet");
-  return false;
+  MachineInstr &MI = *II;
+  MachineFunction &MF = *MI.getParent()->getParent();
+  LLVM_DEBUG(errs() << "\nFunction : " << MF.getName() << "\n";
+             errs() << "<--------->\n"
+                    << MI);
+  int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
+  uint64_t stackSize = MF.getFrameInfo().getStackSize();
+  int64_t spOffset = MF.getFrameInfo().getObjectOffset(FrameIndex);
+  LLVM_DEBUG(errs() << "FrameIndex : " << FrameIndex << "\n" 
+                      << "spOffset   : " << spOffset << "\n" 
+                      << "stackSize  : " << stackSize << "\n");
+  return eliminateFI(MI, FIOperandNum, FrameIndex, stackSize, spOffset);
 }
 
 Register XtensaRegisterInfo::getFrameRegister(const MachineFunction &MF) const {
   const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
   return TFI->hasFP(MF) ? (Xtensa::A15) : Xtensa::SP;
 }

llvm/test/CodeGen/Xtensa/stack-access.ll

+; RUN: llc -mtriple=xtensa -O0 -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=XTENSA
+
+define i8 @loadi8(i8 %a) {
+; XTENSA-LABEL: loadi8:
+; XTENSA: s8i a2, a1, 3
+; XTENSA: l8ui  a2, a1, 3
+; XTENSA: ret
+  %b = alloca i8, align 1
+  store i8 %a, ptr %b, align 1
+  %1 = load i8, ptr %b, align 1
+  ret i8 %1
+}
+
+define i16 @loadi16(i16 %a) {
+; XTENSA-LABEL: loadi16:
+; XTENSA: s16i  a2, a1, 2
+; XTENSA: l16ui a2, a1, 2
+; XTENSA: ret
+  %b = alloca i16, align 2
+    store i16 %a, ptr %b, align 2
+    %1 = load i16, ptr %b, align 2
+  ret i16 %1
+}
+
+define i32 @loadi32(i32 %a) {
+; XTENSA-LABEL: loadi32:
+; XTENSA: s32i  a2, a1, 0
+; XTENSA: l32i  a2, a1, 0
+; XTENSA: ret
+  %b = alloca i32, align 4
+    store i32 %a, ptr %b, align 4
+    %1 = load i32, ptr %b, align 4
+  ret i32 %1
+}
+