[PATCH] D27343: [framelowering] Improve tracking of first CS pop instruction.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

The change itself looks good to me, but would be nice to simplify/cleanup the testcase.



================
Comment at: lib/Target/X86/X86FrameLowering.cpp:1559-1565
     if ((Opc != X86::POP32r || !PI->getFlag(MachineInstr::FrameDestroy)) &&
         (Opc != X86::POP64r || !PI->getFlag(MachineInstr::FrameDestroy)) &&
         Opc != X86::DBG_VALUE && !PI->isTerminator())
       break;
 
+    if (Opc != X86::DBG_VALUE && !PI->isTerminator())
+      FirstCSPop = PI;
----------------
You could use this to avoid code duplication:
`
if (Opc != DBG_VALUE && !Termiantor) {
  if (Opc != Pop32 && Opc != Pop64)
    break;
  FirstCSPop = PI;
}
`


================
Comment at: test/CodeGen/X86/frame-lowering-debug-intrinsic.ll:4
+; RUN: llc  -mtriple=x86_64-unknown-unknown < %s | FileCheck %s
+
+define double @noDebug(double %a) {
----------------
The test seems bigger than necessary. Some things that could be simplified:

- Remove the `; preds =` comments
- Simplify float constants to 0x0? Or use integer math in the first place?
- Do you need the calls to fmod, fabs?
- The nounwind, readnone attributes should not be necessary
- I don't know much about debuginfo, but if there is a way to reduce the amount of metadata at the end...


================
Comment at: test/CodeGen/X86/frame-lowering-debug-intrinsic.ll:23
+
+; CHECK noDebug:
+; CHECK: popq	%rax
----------------
Use `CHECK-LABEL noDebug:` to improve FileCheck output in case of errors.


================
Comment at: test/CodeGen/X86/frame-lowering-debug-intrinsic.ll:46
+
+; CHECK withDebug:
+; CHECK: popq	%rax
----------------
`CHECK-LABEL`


https://reviews.llvm.org/D27343