This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Fix calling windows varargs with floats in fixed args from non-windows functions
ClosedPublic

Authored by mstorsjo on Apr 20 2021, 2:10 PM.

Details

Summary

When inspecting the calling convention, for calling windows functions from a non-windows function, inspect the calling convention of the called function, not the caller.

Also remove an unnecessary parameter to AArch64CallLowering OutgoingArgHandler.

Diff Detail

Event Timeline

mstorsjo created this revision.Apr 20 2021, 2:10 PM
mstorsjo requested review of this revision.Apr 20 2021, 2:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2021, 2:10 PM
Herald added a subscriber: wdng. · View Herald Transcript
mstorsjo added inline comments.Apr 20 2021, 2:24 PM
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
246

This makes the condition a little bit less readable again, undoing @rnk's suggestion - but using UseVarArgsCCForFixed here instead of IsWin64 still is more readable though.

rnk added inline comments.Apr 20 2021, 3:36 PM
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
246

Honestly, I think I was being nit picky. You could calculate it locally:

bool UseVarArgsCCForFixed = IsWin && State.isVarArg();
 if (...)

Right now the IsWin calculation is far from this use of it.

arsenm added inline comments.Apr 20 2021, 5:43 PM
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
170–171

This is also broken. This is using the calling convention of the calling function, not the callee

mstorsjo added inline comments.Apr 21 2021, 12:26 AM
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
170–171

Oh, crap, you're right. And I also have made the same mistake in AArch64ISelLowering. Fix coming up, for both of them. Thanks for noticing it!

mstorsjo updated this revision to Diff 339125.Apr 21 2021, 12:28 AM

Updated to check the CC of the callee, not the caller, both in GlobalISel and AArch64ISelLowering.

mstorsjo retitled this revision from [AArch64] Remove an unnecessary parameter to AArch64CallLowering OutgoingArgHandler. NFC. to [AArch64] Fix calling windows varargs with floats in fixed args from non-windows functions.Apr 21 2021, 12:28 AM
mstorsjo edited the summary of this revision. (Show Details)
arsenm accepted this revision.Apr 21 2021, 5:10 PM
arsenm added inline comments.
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
267

You technically could get this from the function in the builder, but this is fine

This revision is now accepted and ready to land.Apr 21 2021, 5:10 PM