Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dhoekwater
Copy link
Contributor

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 28, 2023

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Hoekwater (dhoekwater)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/76520.diff

4 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/TargetInstrInfo.h (+3-1)
  • (modified) llvm/lib/CodeGen/TargetInstrInfo.cpp (+10-28)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir (+25-27)
  • (modified) llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll (+13-4)
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index a113100f04e621..dc848b92ae681a 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -2081,7 +2081,9 @@ class TargetInstrInfo : public MCInstrInfo {
   /// 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:
diff --git a/llvm/lib/CodeGen/TargetInstrInfo.cpp b/llvm/lib/CodeGen/TargetInstrInfo.cpp
index 4783742a14ad7d..ecfa8df038b1a1 100644
--- a/llvm/lib/CodeGen/TargetInstrInfo.cpp
+++ b/llvm/lib/CodeGen/TargetInstrInfo.cpp
@@ -1778,6 +1778,16 @@ outliner::InstrType TargetInstrInfo::getOutliningType(
     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_EVENT_CALL:
+    case TargetOpcode::PATCHABLE_TYPED_EVENT_CALL:
+    case TargetOpcode::PATCHABLE_RET:
+    case TargetOpcode::PATCHABLE_TAIL_CALL:
+      return outliner::InstrType::Illegal;
     default:
       break;
   }
@@ -1825,31 +1835,3 @@ outliner::InstrType TargetInstrInfo::getOutliningType(
   // 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;
-}
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir b/llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir
index 6e957bceb8f4f8..c9c21faf4953f0 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-patchable.mir
@@ -1,4 +1,5 @@
-# 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
@@ -47,20 +48,16 @@ stack:
 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:
@@ -109,16 +106,10 @@ body:             |
   ; 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:
@@ -150,13 +141,20 @@ body:             |
     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
 
+...
diff --git a/llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll b/llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll
index 4ef3abd241577f..1f3930154a494a 100644
--- a/llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll
+++ b/llvm/test/CodeGen/RISCV/machine-outliner-patchable.ll
@@ -11,7 +11,7 @@ define void @fentry0(i1 %a) nounwind "fentry-call"="true" {
 ; 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:
@@ -27,7 +27,7 @@ define void @fentry1(i1 %a) nounwind "fentry-call"="true" {
 ; 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:
@@ -47,7 +47,7 @@ define void @patchable0(i1 %a) nounwind "patchable-function-entry"="2" {
 ; 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:
@@ -65,7 +65,7 @@ define void @patchable1(i1 %a) nounwind "patchable-function-entry"="2" {
 ; 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:
@@ -75,3 +75,12 @@ 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

@dhoekwater
Copy link
Contributor Author

Previously approved in D156354.

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.

Pull request: llvm#76520
llvm-git-migration pushed a commit to llvm-git-migration/llvm-project that referenced this pull request Feb 13, 2024
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.

Pull request: llvm#76520
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants