[PATCH] D74303: [CFI] cfi directive insertion for callee-save-registers in CFIInstrInserter pass

[Wei Mi via Phabricator via llvm-commits]

wmi marked 8 inline comments as done.
wmi added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:482
+               MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
+    } else if (!MBB.succ_empty() && !MBB.isReturnBlock()) {
+      BuildCFI(MBB, MBBI, DL,
----------------
thegameg wrote:
> Can we just not call this function at all on no-return blocks?
Good point. Done. 


================
Comment at: llvm/test/CodeGen/X86/cfi-epilogue-with-return.mir:9
+  define dso_local i64 @_Z3foob(i1 zeroext %cond) local_unnamed_addr #0 {
+  entry:
+    br i1 %cond, label %if.then, label %return, !prof !2, !misexpect !3
----------------
thegameg wrote:
> I would also remove all the unnecessary IR here if possible.
I remove most of them, but "define _Z3foob" is still needed. In addition, I need to mark the frame-pointer = all attribute to test the cfi related with %rbp is properly set. 


================
Comment at: llvm/test/CodeGen/X86/cfi-epilogue-with-return.mir:55
+  bb.1.if.then:
+    renamable $rbx = MOV64rm $rip, 1, $noreg, @a, $noreg :: (dereferenceable load 8 from `i64* getelementptr inbounds ([10 x i64], [10 x i64]* @a, i64 0, i64 0)`, align 16)
+    renamable $r14 = MOV64rm $rip, 1, $noreg, @a + 8, $noreg :: (dereferenceable load 8 from `i64* getelementptr inbounds ([10 x i64], [10 x i64]* @a, i64 0, i64 1)`)
----------------
thegameg wrote:
> Maybe you could use `renamable $rbx = IMPLICIT_DEF` for all the movs, for a cleaner test here and in the rest of the tests?
Another good point. Done. 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74303/new/

https://reviews.llvm.org/D74303