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

[Kevin B. Smith via llvm-commits]

kbsmith1 added a comment.

Please see inline comments.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:2108
@@ -2107,1 +2107,3 @@
 
+    // Similarly, if we don't have FP, but need to generate unwind information,
+    // we need to set the correct CFA offset at every call site.
----------------
I think this code needs to go after line 2128 in order to create the CFI adjust after the stack adjustment instruction has happened rather than before.

================
Comment at: test/CodeGen/X86/push-cfi-obj.ll:27
@@ +26,3 @@
+; LINUX-NEXT:      0020: 1C000000 24000000 00000000 19000000  |....$...........|
+; LINUX-NEXT:      0030: 04000000 00430E10 2E100020 4D0E1000  |.....C..... M...|
+; LINUX-NEXT:    )
----------------
I don't understand what the specific differences in these values in Section Data coma about from.  Do you know?

================
Comment at: test/CodeGen/X86/push-cfi.ll:14
@@ +13,3 @@
+; LINUX: .cfi_escape 0x2e, 0x10
+; LINUX: .cfi_adjust_cfa_offset 16
+; LINUX-NEXT: pushl   $4
----------------
I don't think this is really the correct spot for the cfi_adjust.  I think it should occur after all the pushes have happenned, so right before the call instruction. Once you get to having a different cfi_adjust per push, each of these should follow immediately after the instruction, not before.  Now I understand that you are not yet getting to the point of emitting a cfi_adjust per instruction, but it seems like even if you are accumulating them so that they are only correct at the call, then they should be after all the pushes.


http://reviews.llvm.org/D13767