Calling convention for arguments on the stack on Xtensa is incorrect for arguments narrower than 32 bits (LLVM-245)

[kyrias]

According to the ISA document function arguments passed on the stack are all essentially stored in 32-bit slots, which matches how GCC for Xtensa behaves. Meanwhile LLVM for Xtensa passes smaller arguments packed together. This means that passing more than 6 arguments to a function between code compiled by GCC and LLVM is currently broken.

E.g.:

GCC:
| 8-bit  | padding                | 16-bit         | padding        | 32-bit                         |

LLVM:
| 8-bit  | 16-bit         | 32-bit                         |

I see in CC_Xtensa_Custom that i8 and i16 are promoted to i32 for non-byval arguments:

llvm-project/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

Lines 744 to 753 in 1e28f73

	// Promote i8 and i16
	if (LocVT == MVT::i8 || LocVT == MVT::i16) {
	LocVT = MVT::i32;
	if (ArgFlags.isSExt())
	LocInfo = CCValAssign::SExt;
	else if (ArgFlags.isZExt())
	LocInfo = CCValAssign::ZExt;
	else
	LocInfo = CCValAssign::AExt;
	}

But the same promotion s not being done for byval ones:

llvm-project/llvm/lib/Target/Xtensa/XtensaISelLowering.cpp

Lines 733 to 742 in 1e28f73

	if (ArgFlags.isByVal()) {
	Align ByValAlign = ArgFlags.getNonZeroByValAlign();
	unsigned Offset = State.AllocateStack(ArgFlags.getByValSize(), ByValAlign);
	State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset, LocVT, LocInfo));
	// Allocate rest of registers, because rest part is not used to pass
	// arguments
	while (State.AllocateReg(IntRegs)) {
	}
	return false;
	}

---

I was considering trying to submit a patch for this, but I'm not sure if you're currently accepting patches for the Xtensa part, or if so how you're accepting them.

[kyrias]

@andreisfr Sorry for the direct ping but it seems like you committed the relevant code. Do you have any idea of what kind of timeline we're looking at to get this fixed? Would be good to have this ABI incompatibility resolved.

[igrr]

This issue has also been reported internally last week a few weeks ago by @MabezDev and the fix is now in progress (internal tracker is LLVM-244). Sorry for not leaving a comment here in time. I think we will have the fix merged this or worst case next week.

[kyrias]

@igrr Ping?

[andreisfr]

Hi @kyrias , we preparing patch now with fixes, I hope in a few days we will update current version.

[andreisfr]

@kyrias , we applied fixes, PTAL.

[kyrias]

@andreisfr The generated code from the xtensa_release_15.x branch looks correct now. 👍

[gerekon]

Should be fixed in df98377 and included into esp-16.0.0-20230515 release.

@kyrias Can we close this issue?