This is an archive of the discontinued LLVM Phabricator instance.

[Arm64EC 7/?] clang side of Arm64EC varargs ABI.
Needs ReviewPublic

Authored by efriedma on May 11 2022, 1:55 PM.

Details

Summary

Part of initial Arm64EC patchset.

Don't try to duplicate the existing logic; instead, just call the actual code we use for native x64.

Diff Detail

Event Timeline

efriedma created this revision.May 11 2022, 1:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2022, 1:55 PM
efriedma requested review of this revision.May 11 2022, 1:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 11 2022, 1:55 PM
efriedma updated this revision to Diff 457459.Sep 1 2022, 5:39 PM
efriedma added a reviewer: rjmccall.
rjmccall added inline comments.Sep 2 2022, 11:11 AM
clang/lib/CodeGen/TargetInfo.cpp
2456

Hmm. Doesn't EC ABI lowering need to preserve this same state, or else you'll get incompatibilities when you exhaust SSE registers?

Should you just be calling over to this at a higher-level point, like in computeInfo?

efriedma added inline comments.Sep 2 2022, 1:54 PM
clang/lib/CodeGen/TargetInfo.cpp
2456

If both IsVectorCall and IsRegCall are false, WinX86_64ABIInfo doesn't use FreeSSERegs, so we don't need to preserve its state. There isn't any other relevant state to preserve.

Return values follow the Arm64 ABI rules, not the x64 rules, so using computeInfo() as an entry point doesn't really make sense.

rjmccall added inline comments.Sep 5 2022, 7:49 PM
clang/lib/CodeGen/TargetInfo.cpp
2456

That's odd. I assume this whole thing is about matching the CC you would get from an instruction-by-instruction translation of x86_64 assembly so that arm64-compiled code can call / be called from translated assembly as long as it uses this special CC. Are there just no functions with interesting return types that anybody happens to care about calling across such a boundary? I would be amazed if the standard arm64 and x64 return rules just happily match for all types.

efriedma planned changes to this revision.Sep 5 2022, 11:04 PM

Just realized I forgot to add tests for va_arg.

clang/lib/CodeGen/TargetInfo.cpp
2456

Normally, arm64ec code uses the arm64 calling convention, and x64 code in the same process uses the x64 calling convention. When there are calls between the two, a thunk translates the calling convention. (See D126811 etc.)

For varargs, things work slightly differently. It's impossible to translate an arbitrary arm64 varargs call to an x64 varargs call, or vice versa, because then thunk doesn't know how the varargs arguments are laid out. (Also, va_list has to be ABI-compatible.) So varargs calls use a special calling convention that ensures varargs arguments in arm64ec calls are laid out exactly the same way as x64 varargs arguments.

There are still thunks for varargs calls, though; among other things, they translates the return value.

efriedma updated this revision to Diff 458290.Sep 6 2022, 3:07 PM

Update to handle va_arg

Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 3:07 PM

I think this looks reasonable to me - if noone else has time to approve it, I guess I could, but I'd rather have the more authoritative reviewers complete their reviews.

llvm/utils/UpdateTestChecks/common.py
330 ↗(On Diff #458290)

I dunno what the practice is for updates to this script - should this change be split out and committed separately? (I guess we don't have tests for the script itself?)

I don't have the expertise to approve, but one question.

Why is there more testing in the .c than in .cpp. Is the same logic being used for both so there's no point checking twice?

If so what things is the .cpp test looking for specifically? I struggle to see how the lines it checks for map into the call to the variadic function. Is it looking at the mangling, the choice of pointer or not?