[XRay] Only avoid outlining pseudo-instructions, not whole blocks

[dhoekwater]

Summary

While some pseudo-instructions, such as fentry and PATCHABLE
instructions, must not be outlined, safe instruction sequences
within the same basic block can be outlined. Move the logic that
prevents pseudo-instruction outlining from the basic block level
to the instruction level. Doing so allows more instructions to be
safely outlined.

[dhoekwater]

Handle PATCHABLE_*EVENT_CALL instructions explicitly

[MaskRay]

Sorry that I'll be away for quite a few days and will be slow to review.

[dhoekwater]

In D156354#4537253, @MaskRay wrote:

Sorry that I'll be away for quite a few days and will be slow to review.

No worries, enjoy your break!

[MaskRay]

LGTM.

llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll

80

CHECK-LABEL:.

It can improve FileCheck diagnostics when things go off. And IIUC, it allows reporting multiple errors after each -LABEL:

87

No newline at end of file

llvm/include/llvm/CodeGen/TargetInstrInfo.h

public:
   /// Returns how or if \p MIT should be outlined. \p Flags is the
   /// target-specific information returned by isMBBSafeToOutlineFrom.
   outliner::InstrType
   getOutliningType(MachineBasicBlock::iterator &MIT, unsigned Flags) const;
 
   /// Optional target hook that returns true if \p MBB is safe to outline from,
   /// and returns any target-specific information in \p Flags.
   virtual bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
-                                      unsigned &Flags) const;
+                                      unsigned &Flags) const {
+    return true;
+  }
 
   /// Optional target hook which partitions \p MBB into outlinable ranges for
   /// instruction mapping purposes. Each range is defined by two iterators:
   /// [start, end).
   ///
   /// Ranges are expected to be ordered top-down. That is, ranges closer to the
   /// top of the block should come before ranges closer to the end of the block.
   ///

llvm/lib/CodeGen/TargetInstrInfo.cpp

outliner::InstrType TargetInstrInfo::getOutliningType(
 
   // Some other special cases.
   switch (MI.getOpcode()) {
     case TargetOpcode::IMPLICIT_DEF:
     case TargetOpcode::KILL:
     case TargetOpcode::LIFETIME_START:
     case TargetOpcode::LIFETIME_END:
       return outliner::InstrType::Invisible;
+
+    // Pseudo-instructions that must not be outlined
+    case TargetOpcode::FENTRY_CALL:
+    case TargetOpcode::PATCHABLE_FUNCTION_ENTER:
+    case TargetOpcode::PATCHABLE_FUNCTION_EXIT:
+    case TargetOpcode::PATCHABLE_RET:
+    case TargetOpcode::PATCHABLE_TAIL_CALL:
+      return outliner::InstrType::Illegal;
     default:
       break;
   }
 
   // Is this a terminator for a basic block?
   if (MI.isTerminator()) {
     // If this is a branch to another block, we can't outline it.
     if (!MI.getParent()->succ_empty())
for (const MachineOperand &MOP : MI.operands()) {
 
     if (MOP.isMBB() || MOP.isBlockAddress() || MOP.isCPI() || MOP.isJTI())
       return outliner::InstrType::Illegal;
   }
 
   // If we don't know, delegate to the target-specific hook.
   return getOutliningTypeImpl(MIT, Flags);
 }
-
-bool TargetInstrInfo::isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
-                                             unsigned &Flags) const {
-  // Some instrumentations create special TargetOpcode at the start which
-  // expands to special code sequences which must be present.
-  auto First = MBB.getFirstNonDebugInstr();
-  if (First == MBB.end())
-    return true;
-
-  if (First->getOpcode() == TargetOpcode::FENTRY_CALL ||
-      First->getOpcode() == TargetOpcode::PATCHABLE_FUNCTION_ENTER)
-    return false;
-
-  // Some instrumentations create special pseudo-instructions at or just before
-  // the end that must be present.
-  auto Last = MBB.getLastNonDebugInstr();
-  if (Last->getOpcode() == TargetOpcode::PATCHABLE_RET ||
-      Last->getOpcode() == TargetOpcode::PATCHABLE_TAIL_CALL)
-    return false;
-
-  if (Last != First && Last->isReturn()) {
-    --Last;
-    if (Last->getOpcode() == TargetOpcode::PATCHABLE_FUNCTION_EXIT ||
-        Last->getOpcode() == TargetOpcode::PATCHABLE_TAIL_CALL)
-      return false;
-  }
-  return true;
-}

llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir

-# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass machine-outliner -verify-machineinstrs -enable-machine-outliner %s -o - | FileCheck %s
+# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass machine-outliner -verify-machineinstrs -enable-machine-outliner %s -o - | FileCheck %s -check-prefixes=CHECK,FUNCTION_A
+# RUN: llc -mtriple=aarch64-none-linux-gnu -run-pass machine-outliner -verify-machineinstrs -enable-machine-outliner %s -o - | FileCheck %s -check-prefixes=CHECK,FUNCTION_B
 --- |
   ; Function Attrs: minsize
   declare void @foo(i32, i32, i32, i32) #0
     
   ; Function Attrs: nounwind
   define void @xray0(i1 %a) #1 {
   entry:
     br i1 %a, label %if.then, label %if.end
liveins:
   - { reg: '$w0', virtual-reg: '' }
 stack:
   - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16, 
       stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true, 
       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 machineFunctionInfo:
   hasRedZone:      false
 body:             |
+  ; COM: Make sure that PATCHABLE_FUNCTION_ENTER and PATCHABLE_FUNCTION_EXIT
+  ; COM: instructions aren't outlined.
   ; CHECK-LABEL: name: xray0
   ; CHECK:       bb.0.entry:
   ; CHECK:         PATCHABLE_FUNCTION_ENTER
   ; CHECK:       bb.1.if.then:
-  ; CHECK:         BL @[[OUTLINED_FUNCTION:OUTLINED_FUNCTION_[0-9]]]
+  ; CHECK:         BL @[[OUTLINED_FUNCTION_A:OUTLINED_FUNCTION_[0-9]]]
   ; CHECK:       bb.2.if.end:
-  ; CHECK-NEXT:    $w0 = MOVZWi 5, 0
-  ; CHECK-NEXT:    $w1 = MOVZWi 6, 0
-  ; CHECK-NEXT:    $w2 = MOVZWi 7, 0
-  ; CHECK-NEXT:    $w3 = MOVZWi 8, 0
-  ; CHECK-NEXT:    BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit killed $w3, implicit-def $sp
-  ; CHECK:         $w0 = MOVZWi 5, 0
-  ; CHECK-NEXT:    $w1 = MOVZWi 6, 0
-  ; CHECK-NEXT:    PATCHABLE_FUNCTION_EXIT
+  ; CHECK:         BL @[[OUTLINED_FUNCTION_B:OUTLINED_FUNCTION_[0-9]]]
+  ; CHECK:         PATCHABLE_FUNCTION_EXIT
   ; CHECK-NEXT:    RET undef $lr
 
   bb.0.entry:
     successors: %bb.1(0x40000000), %bb.2(0x40000000)
     liveins: $w0, $lr
   
     PATCHABLE_FUNCTION_ENTER
     early-clobber $sp = frame-setup STRXpre killed $lr, $sp, -16 :: (store (s64) into %stack.0)
- { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
       debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 machineFunctionInfo:
   hasRedZone:      false
 body:             |
   ; CHECK-LABEL: name: xray1
   ; CHECK:       bb.0.entry:
   ; CHECK:         PATCHABLE_FUNCTION_ENTER
   ; CHECK:       bb.1.if.then:
-  ; CHECK:         BL @[[OUTLINED_FUNCTION]]
+  ; CHECK:         BL @[[OUTLINED_FUNCTION_A]]
   ; CHECK:       bb.2.if.end:
-  ; CHECK-NEXT:    $w0 = MOVZWi 5, 0
-  ; CHECK-NEXT:    $w1 = MOVZWi 6, 0
-  ; CHECK-NEXT:    $w2 = MOVZWi 7, 0
-  ; CHECK-NEXT:    $w3 = MOVZWi 8, 0
-  ; CHECK-NEXT:    BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit killed $w3, implicit-def $sp
-  ; CHECK:         $w0 = MOVZWi 5, 0
-  ; CHECK-NEXT:    $w1 = MOVZWi 6, 0
-  ; CHECK-NEXT:    PATCHABLE_FUNCTION_EXIT
+  ; CHECK:         BL @[[OUTLINED_FUNCTION_B]]
+  ; CHECK:         PATCHABLE_FUNCTION_EXIT
   ; CHECK-NEXT:    RET undef $lr
 
   bb.0.entry:
     successors: %bb.1(0x40000000), %bb.2(0x40000000)
     liveins: $w0, $lr
   
     PATCHABLE_FUNCTION_ENTER
     early-clobber $sp = frame-setup STRXpre killed $lr, $sp, -16 :: (store (s64) into %stack.0)
bb.2.if.end:
     $w3 = MOVZWi 8, 0
     BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit killed $w1, implicit killed $w2, implicit killed $w3, implicit-def $sp
     early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load (s64) from %stack.0)
     $w0 = MOVZWi 5, 0
     $w1 = MOVZWi 6, 0
     PATCHABLE_FUNCTION_EXIT
     RET undef $lr
 
-  ; CHECK: name: [[OUTLINED_FUNCTION]]
-  ; CHECK:     bb.0:
-  ; CHECK:       $w0 = MOVZWi 1, 0
-  ; CHECK-NEXT:  $w1 = MOVZWi 2, 0
-  ; CHECK-NEXT:  $w2 = MOVZWi 3, 0
-  ; CHECK-NEXT:  $w3 = MOVZWi 4, 0
-  ; CHECK-NEXT:  TCRETURNdi @foo, 0, implicit $sp
+  ; FUNCTION_A: name: [[OUTLINED_FUNCTION_A]]
+  ; FUNCTION_A:     bb.0:
+  ; FUNCTION_A:       $w0 = MOVZWi 1, 0
+  ; FUNCTION_A-NEXT:  $w1 = MOVZWi 2, 0
+  ; FUNCTION_A-NEXT:  $w2 = MOVZWi 3, 0
+  ; FUNCTION_A-NEXT:  $w3 = MOVZWi 4, 0
+  ; FUNCTION_A-NEXT:  TCRETURNdi @foo, 0, implicit $sp
+
+  ; FUNCTION_B: name: [[OUTLINED_FUNCTION_B]]
+  ; FUNCTION_B:     bb.0:
+  ; FUNCTION_B:       $w0 = MOVZWi 5, 0
+  ; FUNCTION_B-NEXT:  $w1 = MOVZWi 6, 0
+  ; FUNCTION_B-NEXT:  $w2 = MOVZWi 7, 0
+  ; FUNCTION_B-NEXT:  $w3 = MOVZWi 8, 0
+  ; FUNCTION_B-NEXT:  TCRETURNdi @foo, 0, implicit $sp
 
 ...
-

llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll

 ; RUN: llc < %s -verify-machineinstrs -enable-machine-outliner | FileCheck %s
 
 target triple = "riscv64-unknown-linux-gnu"
 
 declare void @foo(i32, i32, i32, i32) minsize
 
 ;; TargetOpcode::FENTRY_CALL at the start of the function expands to a __fentry__
 ;; call which must be present. Don't outline it.
 define void @fentry0(i1 %a) nounwind "fentry-call"="true" {
 ; CHECK-LABEL: fentry0:
 ; CHECK-NEXT:  # %bb.0:
 ; CHECK-NEXT:    # FEntry call
 ; CHECK:       # %bb.1:
-; CHECK-NEXT:    call t0, OUTLINED_FUNCTION_1
+; CHECK-NEXT:    call t0, [[OUTLINED_FUNCTION:OUTLINED_FUNCTION_[0-9]+]]
 entry:
   br i1 %a, label %if.then, label %if.end
 if.then:
   call void @foo(i32 1, i32 2, i32 3, i32 4)
   br label %if.end
 if.end:
   call void @foo(i32 5, i32 6, i32 7, i32 8)
   ret void
 }
 
 define void @fentry1(i1 %a) nounwind "fentry-call"="true" {
 ; CHECK-LABEL: fentry1:
 ; CHECK-NEXT:  # %bb.0:
 ; CHECK-NEXT:    # FEntry call
 ; CHECK:       # %bb.1:
-; CHECK-NEXT:    call t0, OUTLINED_FUNCTION_1
+; CHECK-NEXT:    call t0, [[OUTLINED_FUNCTION]]
 entry:
   br i1 %a, label %if.then, label %if.end
 if.then:
   call void @foo(i32 1, i32 2, i32 3, i32 4)
   br label %if.end
 if.end:
   call void @foo(i32 5, i32 6, i32 7, i32 8)
   ret void
 }
 
 ;; TargetOpcode::PATCHABLE_FUNCTION_ENTER at the start of the function expands to
 ;; NOPs which must be present. Don't outline them.
 define void @patchable0(i1 %a) nounwind "patchable-function-entry"="2" {
 ; CHECK-LABEL: patchable0:
 ; CHECK-NEXT:  .Lfunc_begin0:
 ; CHECK-NEXT:  # %bb.0:
 ; CHECK-NEXT:    nop
 ; CHECK-NEXT:    nop
 ; CHECK:       # %bb.1:
-; CHECK-NEXT:    call t0, OUTLINED_FUNCTION_1
+; CHECK-NEXT:    call t0, [[OUTLINED_FUNCTION]]
 entry:
   br i1 %a, label %if.then, label %if.end
 if.then:
   call void @foo(i32 1, i32 2, i32 3, i32 4)
   br label %if.end
 if.end:
   call void @foo(i32 5, i32 6, i32 7, i32 8)
   ret void
 }
 
 define void @patchable1(i1 %a) nounwind "patchable-function-entry"="2" {
 ; CHECK-LABEL: patchable1:
 ; CHECK-NEXT:  .Lfunc_begin1:
 ; CHECK-NEXT:  # %bb.0:
 ; CHECK-NEXT:    nop
 ; CHECK-NEXT:    nop
 ; CHECK:       # %bb.1:
-; CHECK-NEXT:    call t0, OUTLINED_FUNCTION_1
+; CHECK-NEXT:    call t0, [[OUTLINED_FUNCTION]]
 entry:
   br i1 %a, label %if.then, label %if.end
 if.then:
   call void @foo(i32 1, i32 2, i32 3, i32 4)
   br label %if.end
 if.end:
   call void @foo(i32 5, i32 6, i32 7, i32 8)
   ret void
 }
+
+;; Make sure that OUTLINED_FUNCTION contains the right instructions
+; CHECK: [[OUTLINED_FUNCTION]]:
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:    li      a0, 1
+; CHECK-NEXT:    li      a1, 2
+; CHECK-NEXT:    li      a2, 3
+; CHECK-NEXT:    li      a3, 4
+; CHECK-NEXT:    jr      t0
 No newline at end of file