[PATCH] D13767: [X86] Fix more -Os + EH issues

David Kreitzer via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 12:02:22 PDT 2015


DavidKreitzer added a comment.

In http://reviews.llvm.org/D13767#272789, @friss wrote:

> In http://reviews.llvm.org/D13767#272344, @rnk wrote:
>
> > In http://reviews.llvm.org/D13767#272094, @mkuper wrote:
> >
> > > In particular, gnu_args_size is now only generated for fp-based CFA. Frederic, does that sounds right to you?
> >
> >
> > Hm, I think there might be an issue here. The cfa_adjust directives don't instruct the unwinder to adjust ESP before transferring to the landingpad, right? They just help indicate where the return address lives in memory.
>
>
> I think you are right, sorry for the confusion. What's the exact semantic of GNU_args_size? Is it only to be used when branching to a landing pad and ignored when just traversing the function?


Yes, exactly. GNU_args_size is the delta between the stack pointer value immediately before the invoke that triggered the exception and the stack pointer value on entry to the landing pad. It is ignored for unwinding purposes.

The value that the unwinder substitutes for %esp/%rsp in CFA expressions & callee-save register location expressions is the value of %esp/%rsp immediately prior to the call/invoke.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2105
@@ -2099,3 +2104,3 @@
 
-    if (HasDwarfEHHandlers && !isDestroy && 
+    if (hasFP(MF) && HasDwarfEHHandlers && !isDestroy &&
         MF.getInfo<X86MachineFunctionInfo>()->getHasPushSequences())
----------------
Why are you suppressing the GNU_ARGS_SIZE directives for !hasFP?  I think they are still needed. If you agree, some corresponding changes will be needed in the tests.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2120
@@ +2119,3 @@
+    if (isDestroy && InternalAmt && DwarfCFI && !hasFP(MF) && 
+        MMI.hasDebugInfo())
+      BuildCFI(MBB, I, DL, 
----------------
It would be nice to encapsulate the hasDebugInfo checks into a separate method, e.g. usePreciseUnwindInfo. That serves a useful documentation purpose and paves the way for supporting asynchronous EH, which also requires precise unwind info.



http://reviews.llvm.org/D13767





More information about the llvm-commits mailing list