[PATCH] D145664: [Xtensa] Add support of the Xtensa function calls

[Matt Arsenault via Phabricator via llvm-commits]

arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: sstefan1.

Needs tests



================
Comment at: llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp:51
+  const XtensaInstrInfo &TII =
+      *static_cast<const XtensaInstrInfo *>(MF.getSubtarget().getInstrInfo());
+
----------------
getSubtarget<XTensaSubtarget.().getInstrInfo


================
Comment at: llvm/lib/Target/Xtensa/XtensaISelLowering.cpp:334
+  // Copy argument values to their designated locations.
+  std::deque<std::pair<unsigned, SDValue>> RegsToPass;
+  SmallVector<SDValue, 8> MemOpChains;
----------------
Why can't this be a regular SmallVector?


================
Comment at: llvm/lib/Target/Xtensa/XtensaISelLowering.cpp:370
+      // floats are passed as right-justified 8-byte values.
+      if (!StackPtr.getNode())
+        StackPtr = DAG.getCopyFromReg(Chain, DL, Xtensa::SP, PtrVT);
----------------
don't need .getNode (here and other places)


================
Comment at: llvm/lib/Target/Xtensa/XtensaISelLowering.cpp:394
+
+  std::string name;
+  unsigned char TF = 0;
----------------
StringRef?


================
Comment at: llvm/lib/Target/Xtensa/XtensaISelLowering.cpp:433-434
+  SmallVector<SDValue, 8> Ops;
+  Ops.push_back(Chain);
+  Ops.push_back(Callee);
+
----------------
Can use initializer list


================
Comment at: llvm/lib/Target/Xtensa/XtensaISelLowering.cpp:472
+    // Copy the value out, gluing the copy to the end of the call sequence.
+    unsigned Reg = VA.getLocReg();
+    SDValue RetValue = DAG.getCopyFromReg(Chain, DL, Reg, VA.getLocVT(), Glue);
----------------
Register / MCRegister instead of unsigned


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp:138
+  *Reg = RegInfo.createVirtualRegister(RC);
+  if (Value >= -2048 && Value <= 2047) {
+    BuildMI(MBB, MBBI, DL, get(Xtensa::MOVI), *Reg).addImm(Value);
----------------
isInt<11>?


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp:140
+    BuildMI(MBB, MBBI, DL, get(Xtensa::MOVI), *Reg).addImm(Value);
+  } else if (Value >= -32768 && Value <= 32767) {
+    int Low = Value & 0xFF;
----------------
isInt<16>?


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp:146
+    BuildMI(MBB, MBBI, DL, get(Xtensa::ADDMI), *Reg).addReg(*Reg).addImm(High);
+  } else if (Value >= -4294967296LL && Value <= 4294967295LL) {
+    // 32 bit arbirary constant
----------------
isInt<32>?


================
Comment at: llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp:85
+  // getFrameRegister() returns.
+  unsigned FrameReg;
+
----------------
MCRegister


================
Comment at: llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp:88
+  if ((FrameIndex >= MinCSFI && FrameIndex <= MaxCSFI))
+    FrameReg = Xtensa::SP;
+  else
----------------
Initialize with ternary operator?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145664/new/

https://reviews.llvm.org/D145664