[llvm] 40bc911 - GlobalISel: Relax handling of G_ASSERT_* with source register classes
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 07:49:59 PDT 2022
Author: Matt Arsenault
Date: 2022-04-22T10:49:50-04:00
New Revision: 40bc9112c079cfdbaa051e55833ec24f64d981a4
URL: https://github.com/llvm/llvm-project/commit/40bc9112c079cfdbaa051e55833ec24f64d981a4
DIFF: https://github.com/llvm/llvm-project/commit/40bc9112c079cfdbaa051e55833ec24f64d981a4.diff
LOG: GlobalISel: Relax handling of G_ASSERT_* with source register classes
The most common situation where G_ASSERT_ZEXT appears for AMDGPU is a
copy from a physical register, which happens to use set the actual
register class on the virtual register. After copy coalescing, the
assert's source operand had a vreg with a set class. The verifier was
strictly rejecting cases where the set class/bank weren't an exact
match. Additionally, RegBankSelect was also expecting a register bank
to be set on the register, not a class.
This is much stricter than regular copies so relax this behavior. This
now allows these 2 cases:
1. Source register has either class or bank, and the result does not
2. Source register has a register class, and the result is a register
with a matching bank.
This should avoid needing some kind of special handling to avoid
violating this constraint when folding copies.
Added:
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-assert-zext.mir
Modified:
llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
llvm/lib/CodeGen/MachineVerifier.cpp
llvm/test/MachineVerifier/test_g_assert_sext_register_bank_class.mir
llvm/test/MachineVerifier/test_g_assert_zext_register_bank_class.mir
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp b/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
index ceaf58db85a31..bce850ee212cf 100644
--- a/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/RegBankSelect.cpp
@@ -630,7 +630,8 @@ bool RegBankSelect::assignInstr(MachineInstr &MI) {
"Unexpected hint opcode!");
// The only correct mapping for these is to always use the source register
// bank.
- const RegisterBank *RB = MRI->getRegBankOrNull(MI.getOperand(1).getReg());
+ const RegisterBank *RB =
+ RBI->getRegBank(MI.getOperand(1).getReg(), *MRI, *TRI);
// We can assume every instruction above this one has a selected register
// bank.
assert(RB && "Expected source register to have a register bank?");
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 36e366351b5cb..eebdfe0e40bd1 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -48,6 +48,7 @@
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/PseudoSourceValue.h"
#include "llvm/CodeGen/RegisterBank.h"
+#include "llvm/CodeGen/RegisterBankInfo.h"
#include "llvm/CodeGen/SlotIndexes.h"
#include "llvm/CodeGen/StackMaps.h"
#include "llvm/CodeGen/TargetInstrInfo.h"
@@ -96,6 +97,7 @@ namespace {
const TargetInstrInfo *TII;
const TargetRegisterInfo *TRI;
const MachineRegisterInfo *MRI;
+ const RegisterBankInfo *RBI;
unsigned foundErrors;
@@ -371,6 +373,7 @@ unsigned MachineVerifier::verify(const MachineFunction &MF) {
TM = &MF.getTarget();
TII = MF.getSubtarget().getInstrInfo();
TRI = MF.getSubtarget().getRegisterInfo();
+ RBI = MF.getSubtarget().getRegBankInfo();
MRI = &MF.getRegInfo();
const bool isFunctionFailedISel = MF.getProperties().hasProperty(
@@ -1001,17 +1004,23 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- if (MRI->getRegBankOrNull(Src) != MRI->getRegBankOrNull(Dst)) {
- report(
- Twine(OpcName, " source and destination register banks must match"),
- MI);
+ const RegisterBank *SrcRB = RBI->getRegBank(Src, *MRI, *TRI);
+ const RegisterBank *DstRB = RBI->getRegBank(Dst, *MRI, *TRI);
+
+ // Allow only the source bank to be set.
+ if ((SrcRB && DstRB && SrcRB != DstRB) || (DstRB && !SrcRB)) {
+ report(Twine(OpcName, " cannot change register bank"), MI);
break;
}
- if (MRI->getRegClassOrNull(Src) != MRI->getRegClassOrNull(Dst))
+ // Don't allow a class change. Do allow member class->regbank.
+ const TargetRegisterClass *DstRC = MRI->getRegClassOrNull(Dst);
+ if (DstRC && DstRC != MRI->getRegClassOrNull(Src)) {
report(
Twine(OpcName, " source and destination register classes must match"),
MI);
+ break;
+ }
break;
}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-assert-zext.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-assert-zext.mir
new file mode 100644
index 0000000000000..95f0bd2208d5d
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-assert-zext.mir
@@ -0,0 +1,102 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=gfx90a -run-pass=regbankselect %s -verify-machineinstrs -o - | FileCheck %s
+
+---
+name: assert_zext_vgpr
+alignment: 4
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0
+
+ ; CHECK-LABEL: name: assert_zext_vgpr
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %copy:vgpr(s32) = COPY $vgpr0
+ ; CHECK-NEXT: %assert_zext:vgpr(s32) = G_ASSERT_ZEXT %copy, 4
+ ; CHECK-NEXT: S_ENDPGM 0, implicit %assert_zext(s32)
+ %copy:_(s32) = COPY $vgpr0
+ %assert_zext:_(s32) = G_ASSERT_ZEXT %copy, 4
+ S_ENDPGM 0, implicit %assert_zext
+...
+
+---
+name: assert_zext_sgpr
+alignment: 4
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $sgpr8
+
+ ; CHECK-LABEL: name: assert_zext_sgpr
+ ; CHECK: liveins: $sgpr8
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %copy:sgpr(s32) = COPY $sgpr8
+ ; CHECK-NEXT: %assert_zext:sgpr(s32) = G_ASSERT_ZEXT %copy, 4
+ ; CHECK-NEXT: S_ENDPGM 0, implicit %assert_zext(s32)
+ %copy:_(s32) = COPY $sgpr8
+ %assert_zext:_(s32) = G_ASSERT_ZEXT %copy, 4
+ S_ENDPGM 0, implicit %assert_zext
+...
+
+---
+name: assert_zext_agpr
+alignment: 4
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $agpr0
+
+ ; CHECK-LABEL: name: assert_zext_agpr
+ ; CHECK: liveins: $agpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %copy:agpr(s32) = COPY $agpr0
+ ; CHECK-NEXT: %assert_zext:agpr(s32) = G_ASSERT_ZEXT %copy, 4
+ ; CHECK-NEXT: S_ENDPGM 0, implicit %assert_zext(s32)
+ %copy:_(s32) = COPY $agpr0
+ %assert_zext:_(s32) = G_ASSERT_ZEXT %copy, 4
+ S_ENDPGM 0, implicit %assert_zext
+...
+
+---
+name: assert_zext_vgpr_regclass
+alignment: 4
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0
+
+ ; CHECK-LABEL: name: assert_zext_vgpr_regclass
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %copy:vgpr_32(s32) = COPY $vgpr0
+ ; CHECK-NEXT: %assert_zext:vgpr(s32) = G_ASSERT_ZEXT %copy, 4
+ ; CHECK-NEXT: S_ENDPGM 0, implicit %assert_zext(s32)
+ %copy:vgpr_32(s32) = COPY $vgpr0
+ %assert_zext:_(s32) = G_ASSERT_ZEXT %copy, 4
+ S_ENDPGM 0, implicit %assert_zext
+...
+
+---
+name: assert_zext_sgpr_regcllass
+alignment: 4
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $sgpr8
+
+ ; CHECK-LABEL: name: assert_zext_sgpr_regcllass
+ ; CHECK: liveins: $sgpr8
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %copy:sgpr_32(s32) = COPY $sgpr8
+ ; CHECK-NEXT: %assert_zext:sgpr(s32) = G_ASSERT_ZEXT %copy, 4
+ ; CHECK-NEXT: S_ENDPGM 0, implicit %assert_zext(s32)
+ %copy:sgpr_32(s32) = COPY $sgpr8
+ %assert_zext:_(s32) = G_ASSERT_ZEXT %copy, 4
+ S_ENDPGM 0, implicit %assert_zext
+...
diff --git a/llvm/test/MachineVerifier/test_g_assert_sext_register_bank_class.mir b/llvm/test/MachineVerifier/test_g_assert_sext_register_bank_class.mir
index 928fea71f3163..7dcb5ff1e9593 100644
--- a/llvm/test/MachineVerifier/test_g_assert_sext_register_bank_class.mir
+++ b/llvm/test/MachineVerifier/test_g_assert_sext_register_bank_class.mir
@@ -3,14 +3,14 @@
name: test
legalized: true
-regBankSelected: true
+regBankSelected: false
body: |
bb.0:
liveins: $w0, $w1
%bank:gpr(s32) = COPY $w0
%class:gpr32(s32) = COPY $w1
- ; CHECK: *** Bad machine code: G_ASSERT_SEXT source and destination register banks must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_SEXT cannot change register bank ***
; CHECK: instruction: %bank_mismatch:fpr(s32) = G_ASSERT_SEXT %bank:gpr, 16
%bank_mismatch:fpr(s32) = G_ASSERT_SEXT %bank, 16
@@ -18,18 +18,14 @@ body: |
; CHECK: instruction: %class_mismatch_gpr:gpr32all(s32) = G_ASSERT_SEXT %class:gpr32, 16
%class_mismatch_gpr:gpr32all(s32) = G_ASSERT_SEXT %class, 16
- ; CHECK: *** Bad machine code: G_ASSERT_SEXT source and destination register classes must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_SEXT cannot change register bank ***
; CHECK: instruction: %class_mismatch_fpr:fpr32(s32) = G_ASSERT_SEXT %class:gpr32, 16
%class_mismatch_fpr:fpr32(s32) = G_ASSERT_SEXT %class, 16
- ; CHECK: *** Bad machine code: G_ASSERT_SEXT source and destination register banks must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_SEXT source and destination register classes must match ***
; CHECK: instruction: %dst_has_class_src_has_bank:gpr32all(s32) = G_ASSERT_SEXT %bank:gpr, 16
%dst_has_class_src_has_bank:gpr32all(s32) = G_ASSERT_SEXT %bank, 16
- ; CHECK: *** Bad machine code: G_ASSERT_SEXT source and destination register banks must match ***
- ; CHECK: instruction: %dst_has_bank_src_has_class:gpr(s32) = G_ASSERT_SEXT %class:gpr32, 16
- %dst_has_bank_src_has_class:gpr(s32) = G_ASSERT_SEXT %class, 16
-
; CHECK: *** Bad machine code: Generic instruction cannot have physical register ***
; CHECK: instruction: %implicit_physreg:gpr(s32) = G_ASSERT_SEXT %class:gpr32, 16, implicit-def $w0
%implicit_physreg:gpr(s32) = G_ASSERT_SEXT %class, 16, implicit-def $w0
diff --git a/llvm/test/MachineVerifier/test_g_assert_zext_register_bank_class.mir b/llvm/test/MachineVerifier/test_g_assert_zext_register_bank_class.mir
index b4ed7162fff30..8c6448e39f5b6 100644
--- a/llvm/test/MachineVerifier/test_g_assert_zext_register_bank_class.mir
+++ b/llvm/test/MachineVerifier/test_g_assert_zext_register_bank_class.mir
@@ -3,14 +3,14 @@
name: test
legalized: true
-regBankSelected: true
+regBankSelected: false
body: |
bb.0:
liveins: $w0, $w1
%bank:gpr(s32) = COPY $w0
%class:gpr32(s32) = COPY $w1
- ; CHECK: *** Bad machine code: G_ASSERT_ZEXT source and destination register banks must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_ZEXT cannot change register bank ***
; CHECK: instruction: %bank_mismatch:fpr(s32) = G_ASSERT_ZEXT %bank:gpr, 16
%bank_mismatch:fpr(s32) = G_ASSERT_ZEXT %bank, 16
@@ -18,18 +18,24 @@ body: |
; CHECK: instruction: %class_mismatch_gpr:gpr32all(s32) = G_ASSERT_ZEXT %class:gpr32, 16
%class_mismatch_gpr:gpr32all(s32) = G_ASSERT_ZEXT %class, 16
- ; CHECK: *** Bad machine code: G_ASSERT_ZEXT source and destination register classes must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_ZEXT cannot change register bank ***
; CHECK: instruction: %class_mismatch_fpr:fpr32(s32) = G_ASSERT_ZEXT %class:gpr32, 16
%class_mismatch_fpr:fpr32(s32) = G_ASSERT_ZEXT %class, 16
- ; CHECK: *** Bad machine code: G_ASSERT_ZEXT source and destination register banks must match ***
+ ; CHECK: *** Bad machine code: G_ASSERT_ZEXT source and destination register classes must match ***
; CHECK: instruction: %dst_has_class_src_has_bank:gpr32all(s32) = G_ASSERT_ZEXT %bank:gpr, 16
%dst_has_class_src_has_bank:gpr32all(s32) = G_ASSERT_ZEXT %bank, 16
- ; CHECK: *** Bad machine code: G_ASSERT_ZEXT source and destination register banks must match ***
- ; CHECK: instruction: %dst_has_bank_src_has_class:gpr(s32) = G_ASSERT_ZEXT %class:gpr32, 16
- %dst_has_bank_src_has_class:gpr(s32) = G_ASSERT_ZEXT %class, 16
-
; CHECK: *** Bad machine code: Generic instruction cannot have physical register ***
; CHECK: instruction: %implicit_physreg:gpr(s32) = G_ASSERT_ZEXT %class:gpr32, 16, implicit-def $w0
%implicit_physreg:gpr(s32) = G_ASSERT_ZEXT %class, 16, implicit-def $w0
+
+ %nothing:_(s32) = G_IMPLICIT_DEF
+
+ ; CHECK: *** Bad machine code: G_ASSERT_ZEXT cannot change register bank ***
+ ; CHECK: %only_dst_has_bank:gpr(s32) = G_ASSERT_ZEXT %nothing:_, 4
+ %only_dst_has_bank:gpr(s32) = G_ASSERT_ZEXT %nothing, 4
+
+ ; CHECK: *** Bad machine code: G_ASSERT_ZEXT cannot change register bank ***
+ ; CHECK: %only_dst_has_class:gpr32all(s32) = G_ASSERT_ZEXT %nothing:_, 4
+ %only_dst_has_class:gpr32all(s32) = G_ASSERT_ZEXT %nothing, 4
More information about the llvm-commits
mailing list