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

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 23 07:28:50 PST 2020


mstorsjo added a comment.

In D87448#2411372 <https://reviews.llvm.org/D87448#2411372>, @hans wrote:

> lgtm
>
> (I added a naming comment, but feel free to resolve that as you feel is best.)

Thanks!

> Relatedly, if one wants to learn about Windows EH, are there any good resources besides studying the code?

https://docs.microsoft.com/en-us/cpp/build/exception-handling-x64 and https://docs.microsoft.com/en-us/cpp/build/arm64-exception-handling have pretty good descriptions of the unwind info formats, https://llvm.org/docs/ExceptionHandling.html describes some things from the IR generator perspective, but I don't know of any good docs of all the other practical details of everything inbetween. (I don't know the things inbetween myself really either, I just try to compare with a known working case, when I need to poke those bits.) For reading/following code, the wine source is sometimes a good reference for things that otherwise isn't documented/visible anywhere - it might not always be exactly correct, but usually is fairly close.



================
Comment at: llvm/lib/MC/MCStreamer.cpp:695
 
+  CurrentProcWinFrameInfoStart = WinFrameInfos.size();
   WinFrameInfos.emplace_back(
----------------
hans wrote:
> When I first saw this variable name I thought it was some kind of offset. But now I see it's really an index into WinFrameInfos.
> 
> Maybe CurrentProcWinFrameInfoIndex would be a better name? And should it be initialized to -1 or some other invalid value rather than 0, since I assume it should always be set here before it's used?
Initializing to -1 should be fine, this is pretty tightly checked to be called in pairs only.

I can change the name, but it isn't necessarily the index of one single frame, but the start index to a range of frames, from startindex to the end of the vector.


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

https://reviews.llvm.org/D87448



More information about the llvm-commits mailing list