[PATCH] D125648: [ARM SEH 6] [ARM] Add SEH opcodes in frame lowering

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D125648#3530563 <https://reviews.llvm.org/D125648#3530563>, @mstorsjo wrote:

> When looking into the tail merging pass, I noticed that if `MachineInstr::isCFIInstruction()` would return true for the `SEH_*` instructions, it could be handled differently and maybe the pass wouldn't break them. But it doesn't seem trivial to test out making that return true for the `SEH_*` instructions, so I don't know if that would fix any of these issues or not.

I managed to make a PoC of that, where the core of the changes were this:

  diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
  index cb6698c12d8e..15bdfdb5d7eb 100644
  --- a/llvm/include/llvm/CodeGen/MachineInstr.h
  +++ b/llvm/include/llvm/CodeGen/MachineInstr.h 
  @@ -112,6 +112,7 @@ public:
       NoMerge      = 1 << 15,             // Passes that drop source location info
                                           // (e.g. branch folding) should skip
                                           // this instruction.
  +    CFILike      = 1 << 16,
     };
   
   private:
  @@ -1207,7 +1208,7 @@ public:
     }
   
     // True if the instruction represents a position in the function.
  -  bool isPosition() const { return isLabel() || isCFIInstruction(); }
  +  bool isPosition() const { return isLabel() || isCFIInstruction() || getFlag(C
  FILike); }
   
     bool isNonListDebugValue() const {
       return getOpcode() == TargetOpcode::DBG_VALUE;
  
  diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding
  .cpp
  index 76f6a00b718e..de81ca874800 100644
  --- a/llvm/lib/CodeGen/BranchFolding.cpp
  +++ b/llvm/lib/CodeGen/BranchFolding.cpp
  @@ -294,7 +294,7 @@ static unsigned HashEndOfMBB(const MachineBasicBlock &MBB) {
   
   /// Whether MI should be counted as an instruction when calculating common tail
  .
   static bool countsAsInstruction(const MachineInstr &MI) {
  -  return !(MI.isDebugInstr() || MI.isCFIInstruction());
  +  return !(MI.isDebugInstr() || MI.isCFIInstruction() || MI.getFlag(MachineInst
  r::CFILike));
   }
   
   /// Iterate backwards from the given iterator \p I, towards the beginning of th
  e

This fixes the issue I was seeing (by setting that flag on all the `SEH_*` instructions). However, I'm not sure if that actually avoids the real issue (of splitting off the `SEH_*` instructions from the regular instructions) or if it just makes the tail merging no longer seem worthwhile doing. It doesn't fix the need to disable the PostRAScheduler in any case.

(Also, adding this new MachineInstr flag is problematic, as it requires widening `MachineInstr::Flags` from `uint16_t` to `uint32_t`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125648