[llvm] [GlobalISel] Insert bitcast instead of register replacement when types don't match. (PR #177397)
Marcos Maronas via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 27 05:31:45 PST 2026
https://github.com/maarquitos14 updated https://github.com/llvm/llvm-project/pull/177397
>From a325949edaa2cfa8c20a9662575b8c6bae1de943 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <mmaronas at amd.com>
Date: Thu, 22 Jan 2026 10:09:46 -0600
Subject: [PATCH 1/5] [GlobalISel] Insert bitcast instead of register
replacement when types don't match.
---
llvm/include/llvm/CodeGen/GlobalISel/Utils.h | 6 +++
.../lib/CodeGen/GlobalISel/CombinerHelper.cpp | 31 +++++++++++-
llvm/lib/CodeGen/GlobalISel/Utils.cpp | 18 +++++++
.../postlegalizer-combiner-merge.mir | 47 +++++++++++++++++++
4 files changed, 100 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-merge.mir
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index da2742e089f8f..6847e3751e487 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -147,6 +147,12 @@ LLVM_ABI bool constrainSelectedInstRegOperands(MachineInstr &I,
LLVM_ABI bool canReplaceReg(Register DstReg, Register SrcReg,
MachineRegisterInfo &MRI);
+/// Check if DstReg can be replaced with SrcReg depending on the register
+/// constraints. Compared to `canReplaceReg`, this does not check types, so
+/// even for registers with different types it can return true.
+LLVM_ABI bool canReplaceRegNoTypeCheck(Register DstReg, Register SrcReg,
+ MachineRegisterInfo &MRI);
+
/// Check whether an instruction \p MI is dead: it only defines dead virtual
/// registers, and doesn't have other side effects.
LLVM_ABI bool isTriviallyDead(const MachineInstr &MI,
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 85d5f06b9813d..f2d649735a9bd 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -2969,8 +2969,35 @@ void CombinerHelper::replaceSingleDefInstWithReg(MachineInstr &MI,
Register Replacement) const {
assert(MI.getNumExplicitDefs() == 1 && "Expected one explicit def?");
Register OldReg = MI.getOperand(0).getReg();
- assert(canReplaceReg(OldReg, Replacement, MRI) && "Cannot replace register?");
- replaceRegWith(MRI, OldReg, Replacement);
+ bool canReplace = canReplaceReg(OldReg, Replacement, MRI);
+ if (canReplace) {
+ replaceRegWith(MRI, OldReg, Replacement);
+ MI.eraseFromParent();
+ return;
+ }
+
+ // `canReplaceReg` can fail because register types don't match, but they can
+ // be _compatible_ (e.g. <2 x s32> and s64). In fact, we expect such a
+ // scenario for some combiner opts (e.g. merge-unmerge), so we need to
+ // explicitly allow mismatched types, and manually check if they are
+ // _compatible_. `canReplaceRegNoTypeCheck` does exactly the same than
+ // `canReplaceReg`, but without type checking. If it still returns false, then
+ // it's something else, and we shouldn't proceed. If it returns true, then we
+ // go on to check type compatibility, and if that passes, we insert a bitcast.
+ assert(canReplaceRegNoTypeCheck(OldReg, Replacement, MRI) &&
+ "Cannot replace register?");
+ LLT SrcType = MRI.getType(OldReg);
+ LLT DstType = MRI.getType(Replacement);
+ // Check if G_BITCAST from SrcType to DstType is legal.
+ LLT BitcastTypes[] = {DstType, SrcType};
+ LegalityQuery Query(TargetOpcode::G_BITCAST, BitcastTypes);
+ // If types are not compatible, we shouldn't proceed.
+ assert(isLegalOrBeforeLegalizer(Query));
+
+ // Build the bitcast from OldReg to Replacement and insert it before MI.
+ Builder.setInstrAndDebugLoc(MI);
+ Builder.buildBitcast(OldReg, Replacement);
+
MI.eraseFromParent();
}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 658774ec3fcb9..41980b070ab89 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -219,6 +219,24 @@ bool llvm::canReplaceReg(Register DstReg, Register SrcReg,
*MRI.getRegClassOrNull(SrcReg));
}
+bool llvm::canReplaceRegNoTypeCheck(Register DstReg, Register SrcReg,
+ MachineRegisterInfo &MRI) {
+ // Give up if either DstReg or SrcReg is a physical register.
+ if (DstReg.isPhysical() || SrcReg.isPhysical())
+ return false;
+ // Replace if either DstReg has no constraints or the register
+ // constraints match.
+ const auto &DstRBC = MRI.getRegClassOrRegBank(DstReg);
+ if (!DstRBC || DstRBC == MRI.getRegClassOrRegBank(SrcReg))
+ return true;
+
+ // Otherwise match if the Src is already a regclass that is covered by the Dst
+ // RegBank.
+ return isa<const RegisterBank *>(DstRBC) && MRI.getRegClassOrNull(SrcReg) &&
+ cast<const RegisterBank *>(DstRBC)->covers(
+ *MRI.getRegClassOrNull(SrcReg));
+}
+
bool llvm::isTriviallyDead(const MachineInstr &MI,
const MachineRegisterInfo &MRI) {
// Instructions without side-effects are dead iff they only define dead regs.
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-merge.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-merge.mir
new file mode 100644
index 0000000000000..5df1ae681d9f0
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/postlegalizer-combiner-merge.mir
@@ -0,0 +1,47 @@
+# RUN: llc -mtriple amdgcn-amd-amdhsa -run-pass=amdgpu-postlegalizer-combiner -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name: merge_unmerge
+alignment: 4
+legalized: true
+liveins:
+body: |
+ bb.1.entry:
+ liveins: $vgpr0_vgpr1
+
+ ; CHECK-LABEL: name: merge_unmerge
+ ; CHECK: liveins: $vgpr0_vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $vgpr0_vgpr1
+ ; CHECK-NEXT: $vgpr0_vgpr1 = COPY [[COPY]](s64)
+ ; CHECK-NEXT: SI_RETURN implicit $vgpr0_vgpr1
+ %0:_(s64) = COPY $vgpr0_vgpr1
+ %a:_(s32), %b:_(s32) = G_UNMERGE_VALUES %0
+ %merge:_(s64) = G_MERGE_VALUES %a, %b
+ $vgpr0_vgpr1 = COPY %merge(s64)
+ SI_RETURN implicit $vgpr0_vgpr1
+
+...
+---
+name: merge_unmerge_vector_type
+alignment: 4
+legalized: true
+liveins:
+body: |
+ bb.1.entry:
+ liveins: $vgpr0_vgpr1
+
+ ; CHECK-LABEL: name: merge_unmerge
+ ; CHECK: liveins: $vgpr0_vgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(<2 x s32>) = COPY $vgpr0_vgpr1
+ ; CHECK-NEXT: [[BITCAST:%[a-z]+]]:_(s64) = G_BITCAST [[COPY]](<2 x s32>)
+ ; CHECK-NEXT: $vgpr0_vgpr1 = COPY [[BITCAST]](s64)
+ ; CHECK-NEXT: SI_RETURN implicit $vgpr0_vgpr1
+ %0:_(<2 x s32>) = COPY $vgpr0_vgpr1
+ %a:_(s32), %b:_(s32) = G_UNMERGE_VALUES %0:_(<2 x s32>)
+ %merge:_(s64) = G_MERGE_VALUES %a:_(s32), %b:_(s32)
+ $vgpr0_vgpr1 = COPY %merge(s64)
+ SI_RETURN implicit $vgpr0_vgpr1
+
+...
>From c7f7333202e7c790812332346d99c58dd4ca389d Mon Sep 17 00:00:00 2001
From: Marcos Maronas <mmaronas at amd.com>
Date: Thu, 22 Jan 2026 10:46:48 -0600
Subject: [PATCH 2/5] Fix clang-format issue.
---
llvm/include/llvm/CodeGen/GlobalISel/Utils.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index 6847e3751e487..5c66754ac7131 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -151,7 +151,7 @@ LLVM_ABI bool canReplaceReg(Register DstReg, Register SrcReg,
/// constraints. Compared to `canReplaceReg`, this does not check types, so
/// even for registers with different types it can return true.
LLVM_ABI bool canReplaceRegNoTypeCheck(Register DstReg, Register SrcReg,
- MachineRegisterInfo &MRI);
+ MachineRegisterInfo &MRI);
/// Check whether an instruction \p MI is dead: it only defines dead virtual
/// registers, and doesn't have other side effects.
>From 123062f35ec67548d3513d393c99d8be7bc00221 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <mmaronas at amd.com>
Date: Tue, 27 Jan 2026 05:41:04 -0600
Subject: [PATCH 3/5] Get rid of wip_match_opcode.
---
.../include/llvm/Target/GlobalISel/Combine.td | 56 +++++++++++++++++--
1 file changed, 52 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index a9b4932b2e317..45c8beffd098b 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -851,10 +851,58 @@ def unmerge_merge : GICombineRule<
// Fold merge(unmerge).
def merge_unmerge : GICombineRule<
- (defs root:$d, register_matchinfo:$matchinfo),
- (match (wip_match_opcode G_MERGE_VALUES):$d,
- [{ return Helper.matchCombineMergeUnmerge(*${d}, ${matchinfo}); }]),
- (apply [{ Helper.replaceSingleDefInstWithReg(*${d}, ${matchinfo}); }])
+ (defs root:$dst, register_matchinfo:$src),
+ (match (G_MERGE_VALUES $dst, GIVariadic<1>:$merge_srcs):$merge,
+ [{
+ assert(!${merge_srcs}.empty() && "GIVariadic<1> should guarantee non-empty.");
+
+ // Check if first source comes from G_UNMERGE_VALUES.
+ Register FirstMergeSrc = ${merge_srcs}[0].getReg();
+ MachineInstr *UnmergeMI = MRI.getVRegDef(FirstMergeSrc);
+
+ if (!UnmergeMI || UnmergeMI->getOpcode() != TargetOpcode::G_UNMERGE_VALUES)
+ return false;
+
+ // Check counts match.
+ unsigned NumMergeSrcs = ${merge_srcs}.size();
+ unsigned NumUnmergeDefs = UnmergeMI->getNumDefs();
+ if (NumMergeSrcs != NumUnmergeDefs)
+ return false;
+
+ // Verify all merge sources match unmerge defs in order.
+ for (unsigned I = 0; I < NumMergeSrcs; ++I) {
+ Register MergeSrc = ${merge_srcs}[I].getReg();
+ Register UnmergeDef = UnmergeMI->getOperand(I).getReg();
+
+ if (MergeSrc != UnmergeDef)
+ return false;
+
+ if (!MRI.hasOneNonDBGUse(MergeSrc))
+ return false;
+ }
+
+ // Get the source of the unmerge.
+ ${src} = UnmergeMI->getOperand(NumUnmergeDefs).getReg();
+
+ // Check size compatibility.
+ LLT SrcTy = MRI.getType(${src});
+ LLT DstTy = MRI.getType(${dst}.getReg());
+ return SrcTy.getSizeInBits() == DstTy.getSizeInBits();
+ }]),
+ (apply [{
+ LLT SrcTy = MRI.getType(${src});
+ LLT DstTy = MRI.getType(${dst}.getReg());
+
+ Helper.getBuilder().setInstrAndDebugLoc(*${merge});
+
+ if (SrcTy == DstTy) {
+ Helper.replaceRegWith(MRI, ${dst}.getReg(), ${src});
+ } else {
+ Helper.getBuilder().buildBitcast(${dst}.getReg(), ${src});
+ }
+
+ ${merge}->eraseFromParent();
+ }])
>;
// Fold (fabs (fneg x)) -> (fabs x).
>From 3c43bfa4ed9c76ddcc07af26bdb623e030a81edd Mon Sep 17 00:00:00 2001
From: Marcos Maronas <mmaronas at amd.com>
Date: Tue, 27 Jan 2026 07:29:32 -0600
Subject: [PATCH 4/5] Undo undesired changes.
---
llvm/include/llvm/CodeGen/GlobalISel/Utils.h | 6 ----
.../lib/CodeGen/GlobalISel/CombinerHelper.cpp | 30 ++-----------------
llvm/lib/CodeGen/GlobalISel/Utils.cpp | 18 -----------
3 files changed, 2 insertions(+), 52 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index 5c66754ac7131..da2742e089f8f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -147,12 +147,6 @@ LLVM_ABI bool constrainSelectedInstRegOperands(MachineInstr &I,
LLVM_ABI bool canReplaceReg(Register DstReg, Register SrcReg,
MachineRegisterInfo &MRI);
-/// Check if DstReg can be replaced with SrcReg depending on the register
-/// constraints. Compared to `canReplaceReg`, this does not check types, so
-/// even for registers with different types it can return true.
-LLVM_ABI bool canReplaceRegNoTypeCheck(Register DstReg, Register SrcReg,
- MachineRegisterInfo &MRI);
-
/// Check whether an instruction \p MI is dead: it only defines dead virtual
/// registers, and doesn't have other side effects.
LLVM_ABI bool isTriviallyDead(const MachineInstr &MI,
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index f2d649735a9bd..3e2d1db3f0475 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -2969,34 +2969,8 @@ void CombinerHelper::replaceSingleDefInstWithReg(MachineInstr &MI,
Register Replacement) const {
assert(MI.getNumExplicitDefs() == 1 && "Expected one explicit def?");
Register OldReg = MI.getOperand(0).getReg();
- bool canReplace = canReplaceReg(OldReg, Replacement, MRI);
- if (canReplace) {
- replaceRegWith(MRI, OldReg, Replacement);
- MI.eraseFromParent();
- return;
- }
-
- // `canReplaceReg` can fail because register types don't match, but they can
- // be _compatible_ (e.g. <2 x s32> and s64). In fact, we expect such a
- // scenario for some combiner opts (e.g. merge-unmerge), so we need to
- // explicitly allow mismatched types, and manually check if they are
- // _compatible_. `canReplaceRegNoTypeCheck` does exactly the same than
- // `canReplaceReg`, but without type checking. If it still returns false, then
- // it's something else, and we shouldn't proceed. If it returns true, then we
- // go on to check type compatibility, and if that passes, we insert a bitcast.
- assert(canReplaceRegNoTypeCheck(OldReg, Replacement, MRI) &&
- "Cannot replace register?");
- LLT SrcType = MRI.getType(OldReg);
- LLT DstType = MRI.getType(Replacement);
- // Check if G_BITCAST from SrcType to DstType is legal.
- LLT BitcastTypes[] = {DstType, SrcType};
- LegalityQuery Query(TargetOpcode::G_BITCAST, BitcastTypes);
- // If types are not compatible, we shouldn't proceed.
- assert(isLegalOrBeforeLegalizer(Query));
-
- // Build the bitcast from OldReg to Replacement and insert it before MI.
- Builder.setInstrAndDebugLoc(MI);
- Builder.buildBitcast(OldReg, Replacement);
+ assert(canReplaceReg(OldReg, Replacement, MRI) && "Cannot replace register?");
+ replaceRegWith(MRI, OldReg, Replacement);
MI.eraseFromParent();
}
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 41980b070ab89..658774ec3fcb9 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -219,24 +219,6 @@ bool llvm::canReplaceReg(Register DstReg, Register SrcReg,
*MRI.getRegClassOrNull(SrcReg));
}
-bool llvm::canReplaceRegNoTypeCheck(Register DstReg, Register SrcReg,
- MachineRegisterInfo &MRI) {
- // Give up if either DstReg or SrcReg is a physical register.
- if (DstReg.isPhysical() || SrcReg.isPhysical())
- return false;
- // Replace if either DstReg has no constraints or the register
- // constraints match.
- const auto &DstRBC = MRI.getRegClassOrRegBank(DstReg);
- if (!DstRBC || DstRBC == MRI.getRegClassOrRegBank(SrcReg))
- return true;
-
- // Otherwise match if the Src is already a regclass that is covered by the Dst
- // RegBank.
- return isa<const RegisterBank *>(DstRBC) && MRI.getRegClassOrNull(SrcReg) &&
- cast<const RegisterBank *>(DstRBC)->covers(
- *MRI.getRegClassOrNull(SrcReg));
-}
-
bool llvm::isTriviallyDead(const MachineInstr &MI,
const MachineRegisterInfo &MRI) {
// Instructions without side-effects are dead iff they only define dead regs.
>From 888d2b2939d97b2fbef527be63282801d7db1704 Mon Sep 17 00:00:00 2001
From: Marcos Maronas <mmaronas at amd.com>
Date: Tue, 27 Jan 2026 07:31:30 -0600
Subject: [PATCH 5/5] Remove unnecessary newline.
---
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 3e2d1db3f0475..85d5f06b9813d 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -2971,7 +2971,6 @@ void CombinerHelper::replaceSingleDefInstWithReg(MachineInstr &MI,
Register OldReg = MI.getOperand(0).getReg();
assert(canReplaceReg(OldReg, Replacement, MRI) && "Cannot replace register?");
replaceRegWith(MRI, OldReg, Replacement);
-
MI.eraseFromParent();
}
More information about the llvm-commits
mailing list