Page MenuHomePhabricator

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

Authored by mstorsjo on Tue, Apr 20, 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.Tue, Apr 20, 2:10 PM
mstorsjo requested review of this revision.Tue, Apr 20, 2:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Apr 20, 2:10 PM
Herald added a subscriber: wdng. · View Herald Transcript
mstorsjo added inline comments.Tue, Apr 20, 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.Tue, Apr 20, 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.Tue, Apr 20, 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.Wed, Apr 21, 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.Wed, Apr 21, 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.Wed, Apr 21, 12:28 AM
mstorsjo edited the summary of this revision. (Show Details)
arsenm accepted this revision.Wed, Apr 21, 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.Wed, Apr 21, 5:10 PM