This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly][MC] Avoid the need for .size directives for functions
ClosedPublic

Authored by sbc100 on Aug 30 2022, 4:38 AM.

Details

Summary

Warn if .size is specified for a function symbol. The size of a
function symbol is determined solely by its content.

I noticed this simplification was possible while debugging #57427, but
this change doesn't fix that specific issue.

Diff Detail

Event Timeline

sbc100 created this revision.Aug 30 2022, 4:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2022, 4:38 AM
Herald added subscribers: pmatos, asb, wingo and 5 others. · View Herald Transcript
sbc100 requested review of this revision.Aug 30 2022, 4:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2022, 4:38 AM
dschuff added inline comments.Aug 30 2022, 2:36 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1702

This isn't for .size, but for the func_end symbol. It looks like (line 1727 below) the func end is stored in a list of MBB section ranges, and MBBSectionRanges is used for debug info generation. Does removing this symbol have an effect on debug info?

1710

It does seem odd that there's a target hook for whether there's a .type/.size directive but we can't use it. Is that just because we want .type (and .size for objects?) but not for functions?

sbc100 updated this revision to Diff 456913.Aug 31 2022, 3:00 AM
  • add comment
sbc100 added inline comments.Aug 31 2022, 3:02 AM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1702

Right, but the func_end only has two possible uses.

  1. needFuncLabelsForEHOrDebugInfo
  2. For calculating the .size

If neither of these is true there is no possible usage of the symbol. So we don't want to both creating this symbol at all except in these two cases.

1710

I added a comment and a new local variable to explain the situation.

dschuff accepted this revision.Aug 31 2022, 11:22 AM
dschuff added inline comments.
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
1702

oh haha right I forgot about the other half of the conditional :D

This revision is now accepted and ready to land.Aug 31 2022, 11:22 AM
This revision was landed with ongoing or failed builds.Aug 31 2022, 2:32 PM
This revision was automatically updated to reflect the committed changes.