[llvm] [AMDGPU][GISel] Notify GISel observers when splitting trap blocks (PR #219128)
Keshav Vinayak Jha via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 01:33:36 PDT 2026
https://github.com/keshavvinayak01 updated https://github.com/llvm/llvm-project/pull/219128
>From c185f642157f43e69ee13350330170fa4d049d62 Mon Sep 17 00:00:00 2001
From: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
Date: Thu, 27 Aug 2026 11:43:48 +0530
Subject: [PATCH 1/3] [AMDGPU] Notify GISel observers when splitting trap
blocks
Splitting at a trap moves following instructions to a new machine basic block. Notify active GISel observers before and after the move so CSE profiles use the new parent block.
Co-authored-by: GPT-5 Codex <noreply at openai.com>
Signed-off-by: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
---
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | 18 ++++++++++++++++++
.../AMDGPU/GlobalISel/legalize-trap.mir | 4 ++++
2 files changed, 22 insertions(+)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 7c1a26f761c96..845a8294ab509 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -24,6 +24,8 @@
#include "SIRegisterInfo.h"
#include "Utils/AMDGPUBaseInfo.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
#include "llvm/CodeGen/GlobalISel/GenericMachineInstrs.h"
#include "llvm/CodeGen/GlobalISel/LegalizerHelper.h"
#include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
@@ -7773,7 +7775,23 @@ bool AMDGPULegalizerInfo::legalizeTrapEndpgm(
// We need a block split to make the real endpgm a terminator. We also don't
// want to break phis in successor blocks, so we can't just delete to the
// end of the block.
+ // An instruction's parent block is part of its CSE profile, so notify
+ // observers about the instructions moved by the split.
+ GISelChangeObserver *Observer = MF->getObserver();
+ SmallVector<MachineInstr *, 8> MovedInstrs;
+ MachineBasicBlock::iterator SplitPoint(&MI);
+ ++SplitPoint;
+ if (Observer && SplitPoint != BB.end()) {
+ for (MachineInstr &MovedMI : make_range(SplitPoint, BB.end())) {
+ Observer->changingInstr(MovedMI);
+ MovedInstrs.push_back(&MovedMI);
+ }
+ }
BB.splitAt(MI, false /*UpdateLiveIns*/);
+ if (Observer) {
+ for (MachineInstr *MovedMI : MovedInstrs)
+ Observer->changedInstr(*MovedMI);
+ }
MachineBasicBlock *TrapBB = MF->CreateMachineBasicBlock();
MF->push_back(TrapBB);
BuildMI(*TrapBB, TrapBB->end(), DL, B.getTII().get(AMDGPU::S_ENDPGM))
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-trap.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-trap.mir
index 80e88e9ed56f0..72567a3783172 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-trap.mir
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-trap.mir
@@ -1,5 +1,6 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
# RUN: llc -mtriple=amdgpu6.00-mesa-mesa3d -run-pass=legalizer -o - %s | FileCheck -check-prefix=GCN %s
+# RUN: %if asserts %{ llc -mtriple=amdgpu6.00-mesa-mesa3d -run-pass=legalizer -enable-cse-in-legalizer=1 -debug-only=cseinfo -filetype=null %s 2>&1 | FileCheck -check-prefix=CSE %s %}
# Check edge cases for trap legalization
@@ -34,6 +35,9 @@ body: |
---
name: test_def_fallthrough_after_trap
body: |
+ ; CSE: CSEInfo::Add MI: %1:_(p1) = G_CONSTANT i64 0
+ ; CSE: CSEInfo::Recording new MI %1:_(p1) = G_CONSTANT i64 0
+ ; CSE: CSEInfo::Recording new MI %1:_(p1) = G_CONSTANT i64 0
; GCN-LABEL: name: test_def_fallthrough_after_trap
; GCN: bb.0:
; GCN-NEXT: successors: %bb.2(0x40000000), %bb.3(0x40000000)
>From 0aea99fae8056be15cf685904f2c49ad5bb9aea3 Mon Sep 17 00:00:00 2001
From: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
Date: Thu, 27 Aug 2026 12:25:39 +0530
Subject: [PATCH 2/3] [AMDGPU] Pass legalizer observer to trap lowering
Pass LegalizerHelper to trap legalization so block-split notifications use the legalizer's required observer directly. This removes the nullable MachineFunction observer lookup and simplifies iteration over moved instructions.
Co-authored-by: GPT-5 Codex <noreply at openai.com>
Signed-off-by: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
---
.../lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | 34 ++++++++-----------
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h | 6 ++--
2 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 845a8294ab509..a794c41c1c86b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -2414,7 +2414,7 @@ bool AMDGPULegalizerInfo::legalizeCustom(
case TargetOpcode::G_SET_FPENV:
return legalizeSetFPEnv(MI, MRI, B);
case TargetOpcode::G_TRAP:
- return legalizeTrap(MI, MRI, B);
+ return legalizeTrap(Helper, MI);
case TargetOpcode::G_DEBUGTRAP:
return legalizeDebugTrap(MI, MRI, B);
default:
@@ -7748,19 +7748,22 @@ bool AMDGPULegalizerInfo::legalizeSBufferPrefetch(LegalizerHelper &Helper,
}
// TODO: Move to selection
-bool AMDGPULegalizerInfo::legalizeTrap(MachineInstr &MI,
- MachineRegisterInfo &MRI,
- MachineIRBuilder &B) const {
+bool AMDGPULegalizerInfo::legalizeTrap(LegalizerHelper &Helper,
+ MachineInstr &MI) const {
+ MachineIRBuilder &B = Helper.MIRBuilder;
+ MachineRegisterInfo &MRI = *B.getMRI();
if (!ST.hasTrapHandler() ||
ST.getTrapHandlerAbi() != GCNSubtarget::TrapHandlerAbi::AMDHSA)
- return legalizeTrapEndpgm(MI, MRI, B);
+ return legalizeTrapEndpgm(Helper, MI);
return ST.supportsGetDoorbellID() ?
legalizeTrapHsa(MI, MRI, B) : legalizeTrapHsaQueuePtr(MI, MRI, B);
}
-bool AMDGPULegalizerInfo::legalizeTrapEndpgm(
- MachineInstr &MI, MachineRegisterInfo &MRI, MachineIRBuilder &B) const {
+bool AMDGPULegalizerInfo::legalizeTrapEndpgm(LegalizerHelper &Helper,
+ MachineInstr &MI) const {
+ MachineIRBuilder &B = Helper.MIRBuilder;
+ GISelChangeObserver &Observer = Helper.Observer;
const DebugLoc &DL = MI.getDebugLoc();
MachineBasicBlock &BB = B.getMBB();
MachineFunction *MF = BB.getParent();
@@ -7777,21 +7780,14 @@ bool AMDGPULegalizerInfo::legalizeTrapEndpgm(
// end of the block.
// An instruction's parent block is part of its CSE profile, so notify
// observers about the instructions moved by the split.
- GISelChangeObserver *Observer = MF->getObserver();
SmallVector<MachineInstr *, 8> MovedInstrs;
- MachineBasicBlock::iterator SplitPoint(&MI);
- ++SplitPoint;
- if (Observer && SplitPoint != BB.end()) {
- for (MachineInstr &MovedMI : make_range(SplitPoint, BB.end())) {
- Observer->changingInstr(MovedMI);
- MovedInstrs.push_back(&MovedMI);
- }
+ for (auto I = std::next(MI.getIterator()), E = BB.end(); I != E; ++I) {
+ Observer.changingInstr(*I);
+ MovedInstrs.push_back(&*I);
}
BB.splitAt(MI, false /*UpdateLiveIns*/);
- if (Observer) {
- for (MachineInstr *MovedMI : MovedInstrs)
- Observer->changedInstr(*MovedMI);
- }
+ for (MachineInstr *MovedMI : MovedInstrs)
+ Observer.changedInstr(*MovedMI);
MachineBasicBlock *TrapBB = MF->CreateMachineBasicBlock();
MF->push_back(TrapBB);
BuildMI(*TrapBB, TrapBB->end(), DL, B.getTII().get(AMDGPU::S_ENDPGM))
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h
index 89819fe990f5a..c68568f775dc7 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h
@@ -251,10 +251,8 @@ class AMDGPULegalizerInfo final : public LegalizerInfo {
bool legalizeSBufferPrefetch(LegalizerHelper &Helper, MachineInstr &MI) const;
- bool legalizeTrap(MachineInstr &MI, MachineRegisterInfo &MRI,
- MachineIRBuilder &B) const;
- bool legalizeTrapEndpgm(MachineInstr &MI, MachineRegisterInfo &MRI,
- MachineIRBuilder &B) const;
+ bool legalizeTrap(LegalizerHelper &Helper, MachineInstr &MI) const;
+ bool legalizeTrapEndpgm(LegalizerHelper &Helper, MachineInstr &MI) const;
bool legalizeTrapHsaQueuePtr(MachineInstr &MI, MachineRegisterInfo &MRI,
MachineIRBuilder &B) const;
bool legalizeTrapHsa(MachineInstr &MI, MachineRegisterInfo &MRI,
>From 92cba26c7b114fe37f9bf09a2bf374b2cd73ca23 Mon Sep 17 00:00:00 2001
From: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
Date: Thu, 27 Aug 2026 13:59:57 +0530
Subject: [PATCH 3/3] [AMDGPU] Fix trap split iterator type
Use an explicit MachineBasicBlock iterator for the range following the trap. This avoids incompatible auto deductions between the raw instruction iterator and the bundle-aware block iterator.
Co-authored-by: GPT-5 Codex <noreply at openai.com>
Signed-off-by: Keshav Vinayak Jha <keshavvinayakjha at gmail.com>
---
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index a794c41c1c86b..7d2bdcb532abb 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -7781,7 +7781,9 @@ bool AMDGPULegalizerInfo::legalizeTrapEndpgm(LegalizerHelper &Helper,
// An instruction's parent block is part of its CSE profile, so notify
// observers about the instructions moved by the split.
SmallVector<MachineInstr *, 8> MovedInstrs;
- for (auto I = std::next(MI.getIterator()), E = BB.end(); I != E; ++I) {
+ MachineBasicBlock::iterator SplitPoint(&MI);
+ ++SplitPoint;
+ for (auto I = SplitPoint, E = BB.end(); I != E; ++I) {
Observer.changingInstr(*I);
MovedInstrs.push_back(&*I);
}
More information about the llvm-commits
mailing list