[clang] [llvm] [AArch64] Support preserve_none calling convention (PR #91046)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 3 20:56:52 PDT 2024


antangelo wrote:

Thank you for catching these.

> X19 is the base register; can we actually allocate arguments in it in general? This seems hard to fix.

I will remove X19 from the argument passing list. `ghccc` also uses X19 for argument passing, and I didn't run into issues while testing this PR, so I'm not sure under what conditions issues present for this but it makes sense to remove it regardless.

> It looks like frame lowering assumes X9 is available; that's probably fixable, but the code needs to be reworked, I think.

The function that allocates X9 as scratch has a fallback provision for when the basic block is not an entry block, where it checks for liveness before picking a scratch register if X9 is unavailable. I think we can modify this so that `preserve_none` functions always follow this path, and then X9 should be usable for argument passing.

> X15 is used on Windows for stack allocation; I think you can use it in this context, but probably worth a test to verify the interaction works the way you want. (And more generally, that this calling convention doesn't explode on Windows targets.)

It looks like X15 is clobbered so I don't think it will be usable on Windows. Do you think it should be excluded from argument passing only on Windows or always?

As an aside, it seems like I missed adding the calling convention to the Windows target in clang, so this is currently only reachable from IR directly.

> What's the interaction between varargs and preserve_none?

I don't believe there is a defined interaction between varargs and `preserve_none` as per the RFC, given that the primary use case is for tail calls (which don't support varargs) I imagine it's unlikely to be used outright. It does not seem to work correctly on either AArch64 with this implementation or the X86_64 reference implementation, so we should probably treat it as unsupported.

I will prepare a patch to address the above issues. Do you think this warrants a revert of this PR in the meantime?

https://github.com/llvm/llvm-project/pull/91046


More information about the llvm-commits mailing list