[PATCH] D125645: [ARM SEH 3] [ARM] [MC] Add support for writing ARM WinEH unwind info

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:2079
   if (Except)
-    OS << ", @except";
+    OS << ", " << Marker << "except";
   EmitEOL();
----------------
mstorsjo wrote:
> What do you think about this use of `%` instead of `@` here, does it seem reasonable?
> 
> The alternative would be to switch to a different comment character, but I'm afraid that would break lots of handwritten assembly out there - while there's very little code that uses these explicit `.seh_handler` directives, so I guess those will just have to cope with some ifdefs.
Switching the character seems fine.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:1431
+    streamer.emitValue(
+        MCBinaryExpr::createOr(FuncLengthExpr,
+                               MCConstantExpr::create(row1, context), context),
----------------
mstorsjo wrote:
> efriedma wrote:
> > mstorsjo wrote:
> > > How do I make this into an ARM specific fixup, which could do range checking for `FuncLengthExpr` and error out if the function is too long?
> > See llvm/lib/Target/ARM/MCTargetDesc/ARMMCExpr.h.
> > 
> > I'd prefer not to put any substantial effort into infrastructure we won't need if we implement splitting, though.  Zhaoshi is currently looking into that.
> Oh, ok. Sure, I'll stop worrying about that aspect then! (As I've never hit that limit in practice on ARM64 so far, I'm not very worried about it for ARM either. But for implementing splitting, the fact that ARM functions often won't have their length fixed at this point might be problematic...)
If we need to, we can put off emitting the unwind info until the layout is fixed.  See the giant FIXME near the beginning of ARM64EmitUnwindInfo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125645