[PATCH] D35844: Correct dwarf unwind information in function epilogue

[Violeta Vukobrat via Phabricator via llvm-commits]

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


================
Comment at: include/llvm/CodeGen/MachineInstr.h:795
 
+  bool isDirective() const { return isDebugValue() || isCFIInstruction(); }
   bool isPHI() const {
----------------
iteratee wrote:
> MatzeB wrote:
> > violetav wrote:
> > > MatzeB wrote:
> > > > This name is unintuitive to me. And it feels like this isn't really a category of machine instructions but rather just a combination that `BranchFolding` is able to deal with after this patch.
> > > > 
> > > > Maybe it would be better to move this into BranchFolding.cpp then in the form of
> > > > `static bool isDirective(const MachineInstr &MI) { return MI.isDebugValue() || MI.isCFIInstruction(); }`.
> > > This is used in BranchFolding and TailDuplication. It was suggested previously to be done this way https://reviews.llvm.org/D18046?id=101198#inline-294697
> > @iteratee Have you seen isMetaInstruction() and isTransient()? This feels like yet another variation of machine instructions that don't produce code. However I don't know why this specific combination is interesting and the generic name and no comment doesn't help understanding that either.
> I hadn't.
> Violeta, are there any reasons those 2 categories won't work?
Four regression tests fail when isDirective() is replaced with isMetaInstruction():
avx512-mask-op.ll
conditional-tailcall.ll
extra-toc-reg-deps.ll
tail-dup-merge-loop-headers.ll

Different code gets generated because of some tail merging and tail duplicating that don't happen with clean version of the code. At first glance, it seems that presence of IMPLICIT_DEF affects BranchFolding, and presence of KILL affects TailDuplicator. I haven't looked at this further than comparing .s files.


Repository:
  rL LLVM

https://reviews.llvm.org/D35844