[Xtensa] Implement vararg support. #117126

andreisfr · 2024-11-21T08:19:07Z

No description provided.

arsenm · 2024-11-27T23:11:58Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+    unsigned RegSize = 4;
+    MVT RegTy = MVT::getIntegerVT(RegSize * 8);


Just use MVT::i32 directly?

arsenm · 2024-11-27T23:12:10Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+    XtensaFI->setVarArgsFirstGPR(Idx + 2); // 2 - number of a2 register
+
+    XtensaFI->setVarArgsStackOffset(MFI.CreateFixedObject(
+        PtrVT.getSizeInBits() / 8, CCInfo.getStackSize(), true));


getStoreSize

arsenm · 2024-11-27T23:12:54Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+      cast<StoreSDNode>(Store.getNode())
+          ->getMemOperand()
+          ->setValue((Value *)nullptr);


You shouldn't be adjusting this after the creation of the store

arsenm · 2024-11-27T23:13:06Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+    // to the vararg save area.
+    for (unsigned I = Idx; I < ArgRegs.size(); ++I, VaArgOffset += RegSize) {
+      const unsigned Reg = RegInfo.createVirtualRegister(RC);
+      unsigned FrameReg = Subtarget.getRegisterInfo()->getFrameRegister(MF);


use Register, and pull out of loop

I removed redundant code. Fixed.

arsenm · 2024-11-27T23:13:43Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

@@ -304,13 +318,14 @@ SDValue XtensaTargetLowering::LowerFormalArguments(
    SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {
  MachineFunction &MF = DAG.getMachineFunction();
  MachineFrameInfo &MFI = MF.getFrameInfo();
+  XtensaMachineFunctionInfo *XtensaFI = MF.getInfo<XtensaMachineFunctionInfo>();
+  EVT PtrVT = getPointerTy(MF.getDataLayout());


In context this probably should be getFrameIndexTy

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

arsenm · 2024-12-04T21:13:22Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

        report_fatal_error("RegVT not supported by FormalArguments Lowering");

      // Transform the arguments stored on
      // physical registers into virtual ones
-      unsigned Register = MF.addLiveIn(VA.getLocReg(), RC);
+      unsigned Register = MF.addLiveIn(VA.getLocReg(), &Xtensa::ARRegClass);


Suggested change

unsigned Register = MF.addLiveIn(VA.getLocReg(), &Xtensa::ARRegClass);

Register Reg = MF.addLiveIn(VA.getLocReg(), &Xtensa::ARRegClass);

Use Register type and try to avoid shadowing type names with variable names

Thank you very much for comments. Fixed.

arsenm · 2024-12-04T21:14:21Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

-    static const MCPhysReg XtensaArgRegs[6] = {
-        Xtensa::A2, Xtensa::A3, Xtensa::A4, Xtensa::A5, Xtensa::A6, Xtensa::A7};
-    ArrayRef<MCPhysReg> ArgRegs = ArrayRef(XtensaArgRegs);
+    ArrayRef<MCPhysReg> ArgRegs = ArrayRef(IntRegs);
    unsigned Idx = CCInfo.getFirstUnallocated(ArgRegs);


Directly passing IntRegs to getFirstUnallocated should work I think

Fix variable names in XtensaMachineFunctionInfo class. Fix LowerFormalArguments function.

arsenm · 2024-12-04T22:09:07Z

llvm/lib/Target/Xtensa/XtensaMachineFunctionInfo.h

-  int VarArgsStackOffset;
-  unsigned VarArgsFrameIndex;
+  unsigned VarArgsOnStackFrameIndex;
+  unsigned VarArgsInRegsFrameIndex;


Suggested change

unsigned VarArgsInRegsFrameIndex;

int VarArgsInRegsFrameIndex;

Frame indexes are signed

arsenm · 2024-12-04T22:09:17Z

llvm/lib/Target/Xtensa/XtensaMachineFunctionInfo.h

@@ -43,12 +44,18 @@ class XtensaMachineFunctionInfo : public MachineFunctionInfo {
  unsigned getVarArgsFirstGPR() const { return VarArgsFirstGPR; }
  void setVarArgsFirstGPR(unsigned GPR) { VarArgsFirstGPR = GPR; }

-  int getVarArgsOnStackFrameIndex() const { return VarArgsStackOffset; }
-  void setVarArgsOnStackFrameIndex(int Offset) { VarArgsStackOffset = Offset; }
+  unsigned getVarArgsOnStackFrameIndex() const {


Suggested change

unsigned getVarArgsOnStackFrameIndex() const {

int getVarArgsOnStackFrameIndex() const {

arsenm · 2024-12-04T22:09:25Z

llvm/lib/Target/Xtensa/XtensaMachineFunctionInfo.h

+  unsigned getVarArgsOnStackFrameIndex() const {
+    return VarArgsOnStackFrameIndex;
+  }
+  void setVarArgsOnStackFrameIndex(unsigned FI) {


Suggested change

void setVarArgsOnStackFrameIndex(unsigned FI) {

void setVarArgsOnStackFrameIndex(int FI) {

Thank you. Fixed.

arsenm · 2024-12-04T22:09:38Z

llvm/lib/Target/Xtensa/XtensaMachineFunctionInfo.h

+  unsigned getVarArgsInRegsFrameIndex() const {
+    return VarArgsInRegsFrameIndex;
+  }
+  void setVarArgsInRegsFrameIndex(unsigned FI) { VarArgsInRegsFrameIndex = FI; }


Suggested change

unsigned getVarArgsInRegsFrameIndex() const {

return VarArgsInRegsFrameIndex;

}

void setVarArgsInRegsFrameIndex(unsigned FI) { VarArgsInRegsFrameIndex = FI; }

int getVarArgsInRegsFrameIndex() const {

return VarArgsInRegsFrameIndex;

}

void setVarArgsInRegsFrameIndex(int FI) { VarArgsInRegsFrameIndex = FI; }

arsenm · 2024-12-04T23:02:29Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+
+  SDValue StkIndex =
+      DAG.getNode(ISD::ADD, DL, PtrVT, VAIndex,
+                  DAG.getConstant(32 + VASizeInBytes, DL, MVT::i32));


Maybe should use getObjectPtrOffset in a follow up for all of these adds

I added use of getObjectPtrOffset to access vararg struct fields.

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

arsenm · 2024-12-06T14:51:55Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+  Align ArgAlignment = TD.getPrefTypeAlign(VT.getTypeForEVT(*DAG.getContext()));
+  unsigned ArgAlignInBytes = ArgAlignment.value();
+  unsigned ArgSizeInBytes =
+      TD.getTypeAllocSize(VT.getTypeForEVT(*DAG.getContext()));


Don't repeat getTypeForEVT twice. I also find it more confusing to use query the type alloc size and the alignment. If you need the explicit alignment, do the alignment yourself

llvm/test/CodeGen/Xtensa/vararg.ll

s-barannikov · 2024-12-10T23:40:47Z

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

+    OrigIndex = DAG.getNode(ISD::ADD, DL, PtrVT, OrigIndex,
+                            DAG.getConstant(ArgAlignInBytes - 1, DL, MVT::i32));
+    OrigIndex = DAG.getNode(ISD::AND, DL, PtrVT, OrigIndex,
+                            DAG.getConstant(-ArgAlignInBytes, DL, MVT::i32));


This will trigger an assertion. Signed constants should be created with getSignedConstant.

Thank you for comment. Fixed.

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

[Xtensa] Implement variable arguments support.

d3db27e

andreisfr force-pushed the xtensa_variable_args branch from 174b461 to d3db27e Compare November 27, 2024 22:00

andreisfr requested review from s-barannikov and arsenm November 27, 2024 22:26

arsenm reviewed Nov 27, 2024

View reviewed changes

arsenm added the backend:Xtensa label Nov 27, 2024

s-barannikov reviewed Nov 28, 2024

View reviewed changes

gerekon mentioned this pull request Nov 28, 2024

Tracking Issue for merging to upstream (LLVM-57) espressif/llvm-project#4

Open

[Xtensa] Minor fixes.

467e1d0

arsenm reviewed Dec 4, 2024

View reviewed changes

[Xtensa] Minor fixes.

dfb7ee4

Fix variable names in XtensaMachineFunctionInfo class. Fix LowerFormalArguments function.

arsenm reviewed Dec 4, 2024

View reviewed changes

[Xtensa] Fix XtensaMachineFunctionInfo.

3998c2f

arsenm approved these changes Dec 4, 2024

View reviewed changes

[Xtensa] Minor fixes in lowering VASTART/VAARG.

862f1de

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp Outdated Show resolved Hide resolved

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp Outdated Show resolved Hide resolved

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp Outdated Show resolved Hide resolved

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp Outdated Show resolved Hide resolved

arsenm reviewed Dec 6, 2024

View reviewed changes

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/test/CodeGen/Xtensa/vararg.ll Show resolved Hide resolved

s-barannikov reviewed Dec 6, 2024

View reviewed changes

llvm/test/CodeGen/Xtensa/vararg.ll Show resolved Hide resolved

[Xtensa] Fix lowering VACOPY/VAARG.

ae7bff0

andreisfr force-pushed the xtensa_variable_args branch from b0f50ea to ae7bff0 Compare December 9, 2024 23:54

gerekon mentioned this pull request Dec 10, 2024

[Clang][Xtensa] Add Xtensa target. #118008

Merged

andreisfr requested a review from s-barannikov December 10, 2024 23:29

s-barannikov reviewed Dec 10, 2024

View reviewed changes

s-barannikov approved these changes Dec 10, 2024

View reviewed changes

arsenm reviewed Dec 11, 2024

View reviewed changes

llvm/lib/Target/Xtensa/XtensaISelLowering.cpp Outdated Show resolved Hide resolved

[Xtensa] Minor fixes.

90eaaa4

arsenm approved these changes Dec 11, 2024

View reviewed changes

andreisfr merged commit be4a183 into llvm:main Dec 12, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xtensa] Implement vararg support. #117126

[Xtensa] Implement vararg support. #117126

andreisfr commented Nov 21, 2024

arsenm Nov 27, 2024

andreisfr Dec 4, 2024

arsenm Nov 27, 2024

andreisfr Dec 4, 2024

arsenm Nov 27, 2024

andreisfr Dec 4, 2024

arsenm Nov 27, 2024

andreisfr Dec 4, 2024

arsenm Nov 27, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 4, 2024

arsenm Dec 4, 2024

andreisfr Dec 6, 2024

arsenm Dec 6, 2024

andreisfr Dec 9, 2024

s-barannikov Dec 10, 2024

andreisfr Dec 11, 2024

		unsigned RegSize = 4;
		MVT RegTy = MVT::getIntegerVT(RegSize * 8);

	unsigned Register = MF.addLiveIn(VA.getLocReg(), &Xtensa::ARRegClass);
	Register Reg = MF.addLiveIn(VA.getLocReg(), &Xtensa::ARRegClass);

	unsigned VarArgsInRegsFrameIndex;
	int VarArgsInRegsFrameIndex;

	unsigned getVarArgsOnStackFrameIndex() const {
	int getVarArgsOnStackFrameIndex() const {

	void setVarArgsOnStackFrameIndex(unsigned FI) {
	void setVarArgsOnStackFrameIndex(int FI) {

[Xtensa] Implement vararg support. #117126

[Xtensa] Implement vararg support. #117126

Conversation

andreisfr commented Nov 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment