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

Reid Kleckner via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 19 15:57:00 PDT 2015


rnk added inline comments.

================
Comment at: lib/Target/X86/X86CallFrameOptimization.cpp:503
@@ +502,3 @@
+    if (MF.getMMI().hasDebugInfo())
+      static_cast<const X86FrameLowering *>(TFL)->BuildCFI(MBB, std::next(Push), DL, 
+                MCCFIInstruction::createAdjustCfaOffset(nullptr, 4));
----------------
mkuper wrote:
> Argh, this should have been a cast<>, not a static_cast<>.
> Also, I can avoid this completely and copy the code from the BuildCFI() helper, but I think it's better this way.
You could also change TFL to hold an X86FrameLowering. If you ask X86Subtarget for the framelowering, it actually uses covariant return types to return you an X86FrameLowering instead of a TargetFrameLowering.

================
Comment at: test/CodeGen/X86/push-cfi-debug.ll:17-21
@@ +16,7 @@
+; CHECK: .cfi_adjust_cfa_offset -16
+define void @test1() #0 {
+entry:
+  tail call void @foo(i32 1, i32 2) #1, !dbg !10
+  ret void, !dbg !11
+}
+
----------------
Can you add a test for a callee-cleanup convention like stdcall? I think with the code as written we'll get something like this on Linux, where the stack is 16-byte aligned:
  subl $4, %esp
  .cfi_adjust_cfa_offset 4
  pushl $3
  .cfi_adjust_cfa_offset 4
  pushl $2
  .cfi_adjust_cfa_offset 4
  pushl $1
  .cfi_adjust_cfa_offset 4
  calll _bar
  addl $4, %esp
  .cfi_adjust_cfa_offset -16
Which isn't *strictly* speaking correct, it should be:
  calll bar
  .cfi_adjust_cfa_offset -12
  addl $4, %esp
  .cfi_adjust_cfa_offset -4
We don't need to fix the issue in this change, I just like having test cases with FIXMEs.


http://reviews.llvm.org/D13767





More information about the llvm-commits mailing list