This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Replace CCState's getNextStackOffset with getStackSize (NFC)
ClosedPublic

Authored by barannikov88 on Apr 30 2023, 7:44 PM.

Details

Summary

The term "next stack offset" is misleading because the next argument is
not necessarily allocated at this offset due to alignment constrains.
It also does not make much sense when allocating arguments at negative
offsets (introduced in a follow-up patch), because the returned offset
would be past the end of the next argument.

Diff Detail

Unit TestsFailed

Event Timeline

barannikov88 created this revision.Apr 30 2023, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2023, 7:44 PM
barannikov88 edited the summary of this revision. (Show Details)Apr 30 2023, 7:45 PM

Self-review fixes

clang-format

Disregard clang-format broken suggestions

barannikov88 published this revision for review.May 1 2023, 4:08 AM
arsenm added inline comments.May 12 2023, 7:34 AM
llvm/include/llvm/CodeGen/CallingConvLower.h
178

Maybe CurrentStackSize would help make it clearer this is a stateful thing which may keep increasing

barannikov88 added inline comments.May 12 2023, 8:25 AM
llvm/include/llvm/CodeGen/CallingConvLower.h
178

I can rename this variable and/or add a comment (there is an equivalent in GISel),
but I wouldn't want to change the name of getStackSize() and getAlignedCallFrameSize().
Most clients use them when the stack has finished growing. As for other uses, I think it shouldn't
cause much confusion; there is std::my_fav_container::size() that also returns the current size.
What do you think?

arsenm accepted this revision.May 12 2023, 11:15 AM
This revision is now accepted and ready to land.May 12 2023, 11:15 AM
barannikov88 added a subscriber: dmgreen.

@dmgreen Could you take a look? This renames some local variables in ARM/AArch64, other targets are less affected.

This revision was landed with ongoing or failed builds.May 17 2023, 11:52 AM
This revision was automatically updated to reflect the committed changes.