[PATCH] D87448: [CodeGen] [WinException] Only produce handler data at the end of the function if needed

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D87448#2266746 <https://reviews.llvm.org/D87448#2266746>, @rnk wrote:

> Makes sense.
>
>> Unwind info for all remaining functions that hasn't gotten it emitted directly is emitted at the end.
>
> Can we make `.seh_endproc` responsible for emitting the unwind info into `.xdata` if necessary?

I guess it could be done, but... Right now, EmitWinCFIEndProc is only implemented in the MCStreamer base class, while the class responsible for actually writing the .xdata contents, Win64EH::UnwindEmitter or Win64EH::ARM64UnwindEmitter) is in the subclasses X86WinCOFFStreamer and AArch64WinCOFFStreamer, so it'd require overriding EmitWinCFIEndProc in these subclasses and calling the respective UnwindEmitter methods for writing the xdata right there. Not hard, but a bit extra boilerplate needed in any target specific subclass doing this, if they're to be consistent wrt this.

> Would that preserve the section ordering? There is something nice about having all the .text/.pdata/.xdata sections grouped together in the object file, even though the order doesn't matter.

I guess it would - but even right now, they're not grouped together right now. For an object file with two comdat functions, func1 and func2, GCC/binutils do generate them in the order .text$func1, .xdata$func1, .pdata$func1, .text$func2, .xdata$func2, .pdata$func2 - i.e .text/.xdata/.pdata ordered together.

Currently, LLVM generates .text$func1, .xdata$func1, .text$func2, .xdata$func2, .pdata$func1, .pdata$func2 - i.e. .text/.xdata pairwise together, but all .pdata emitted at the end. With this patch in place, it generates .text$func1, .text$func2, .xdata$func1, .xdata$func2, .pdata$func1, .pdata$func2, which admittedly is the least nice ordering.

In that sense, the current ordering isn't much to strive for either, so if we want to make an effort towards keeping that, we probably also should emit the .pdata right next after the corresponding .xdata. I don't think that's much extra work either, but still a little bigger refactoring than just "stop emitting unnecessary .seh_handlerdata and keep .xdata where it was".



================
Comment at: llvm/test/CodeGen/X86/mingw-comdats.ll:81
 ; GNUOBJ: .xdata$_Z3fooi
-; GNUOBJ: .data$gv
 ; GNUOBJ: .pdata$_Z3fooi
----------------
rnk wrote:
> Shouldn't this remain? This change seems unrelated.
It was a mistake on my side; it does remain, but the sections have changed order, and I thought it wasn't important to the testcase (like unrelated lines in the middle of a CHECK-NEXT sequence), but I see it's part of what it actually tries to check. I've locally updated the patch to move it up between text and xdata.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87448