[PATCH] D125536: [MC] [MCWin64EH] Try writing an ARM64 "packed epilog" even if the epilog doesn't share opcodes with the prolog

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:656
 
   info->EpilogMap.clear();
   return Offset;
----------------
This call to `EpilogMap.clear()` tripped me up in trying to understand the reasoning behind the rest of the changes in this patch.  There are three cases here:

1. Unpacked epilogues
2. Packed epilogue reusing the prologue
3. Packed epilogue not reusing the prologue.

In the "Packed epilogue reusing the prologue" case, we clear EpilogMap; in the other cases, we don't.

Maybe the logic here could be a bit more explicit?  Like maybe instead of clearing EpilogMap(), add checks in the caller in the two places it matters.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:990
     MCSymbol* MatchingEpilog =
       FindMatchingEpilog(EpilogInstrs, AddedEpilogs, info);
     if (MatchingEpilog) {
----------------
Possible followup optimization: we could teach FindMatchingEpilog to look at the prologue.  (Not sure how frequently that would trigger.)


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:1029
     // FIXME: We should share epilog codes across epilogs, where possible,
     // which would make this issue show up less frequently.
     if (CodeWords > 0xFF || EpilogCount > 0xFFFF)
----------------
Is the fixme about sharing epilog codes fixed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125536/new/

https://reviews.llvm.org/D125536