This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Add MachineFunction::estimateSizeInBytes()
AbandonedPublic

Authored by piggynl on Jul 17 2022, 2:56 PM.

Details

Summary

This will avoid duplicating EstimateFunctionSizeInBytes for architectures.

Diff Detail

Event Timeline

piggynl created this revision.Jul 17 2022, 2:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2022, 2:56 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
piggynl requested review of this revision.Jul 17 2022, 2:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2022, 2:56 PM
piggynl retitled this revision from [llvm] Add MachineFunction::estimateSizeInBytes() to [CodeGen] Add MachineFunction::estimateSizeInBytes().Jul 17 2022, 2:57 PM
piggynl updated this revision to Diff 445355.Jul 17 2022, 3:15 PM

Fix compile error.

Looks fine to me, but I'm not active in this part of the codebase, it would be better for others to review it.

piggynl planned changes to this revision.Jul 19 2022, 6:35 AM

ARM hard-coded EK_Inline as jump table encoding in rG8d3ba7349c2137f1a7f68c0ce5c1345496fc0b63. As a result, std::max(JTI->getEntrySize(DL), JTI->getEntryAlignment(DL)) always returns 1, which causes test/CodeGen/Thumb/large-fn-switch.ll to fail.

zixuan-wu added inline comments.Jul 19 2022, 7:02 PM
llvm/lib/Target/CSKY/CSKYFrameLowering.cpp
274

CSKY function does not contain jumptable now if there is any jumpable.

The ARM version EstimateFunctionSizeInBytes in the context of frame lowering needs to ensure that it interacts correctly with later passes that might cause the function to grow, in particular ARMConstantIslandPass. The inclusion of constant pools and jump tables is intended to account for that. On targets which don't have an equivalent pass, you probably don't want to measure them. I guess CSKY in particular does embed constant pools... but it doesn't look like it embeds jump tables?

Anyway, it isn't obvious to me that you can actually come up with an estimate that actually makes sense in a target-independent context, so I'm not sure this is worth doing.

piggynl abandoned this revision.Jul 26 2022, 8:52 PM