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

Michael Kuperstein via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 23:59:21 PDT 2015


mkuper added a comment.

Thanks, everyone. Looks like I got confused about the semantics, will upload a new patch.
Sorry again for the amount of noise this is causing.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2105
@@ -2099,3 +2104,3 @@
 
-    if (HasDwarfEHHandlers && !isDestroy && 
+    if (hasFP(MF) && HasDwarfEHHandlers && !isDestroy &&
         MF.getInfo<X86MachineFunctionInfo>()->getHasPushSequences())
----------------
DavidKreitzer wrote:
> 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.
> 
Yes, this is a mistake, I misinterpreted Frederic's comment. Will make the appropriate changes, thanks.

================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2120
@@ +2119,3 @@
+    if (isDestroy && InternalAmt && DwarfCFI && !hasFP(MF) && 
+        MMI.hasDebugInfo())
+      BuildCFI(MBB, I, DL, 
----------------
DavidKreitzer wrote:
> 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.
> 
Right, will do.


http://reviews.llvm.org/D13767





More information about the llvm-commits mailing list