[llvm] SelectionDAG store merging default implementations are unreasonable #90714 (PR #131424)
Devesh Yadav via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 15 07:34:28 PDT 2025
https://github.com/Devesh-Yadav10 updated https://github.com/llvm/llvm-project/pull/131424
>From 452a6e136c601196ba55bca5a512ab85f2ddcf07 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 08:44:11 +0530
Subject: [PATCH 01/10] Updated unconditional true in
mergeStoresAfterLegalization to checking isOperationLegal(ISD::STORE, MemVT)
---
llvm/include/llvm/CodeGen/TargetLowering.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index a3fb4e9a8513b..a52dcf68dd243 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -695,7 +695,7 @@ class TargetLoweringBase {
/// to before legalization. This may transform stores that do not exist
/// earlier (for example, stores created from intrinsics).
virtual bool mergeStoresAfterLegalization(EVT MemVT) const {
- return true;
+ return isOperationLegal(ISD::STORE, MemVT);
}
/// Returns if it's reasonable to merge stores to MemVT size.
>From 24c8a57695adecbcb2036773f98827dea050c08f Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:27:02 +0530
Subject: [PATCH 02/10] [GlobalISel] Introduce G_POISON to distinguish poison
from undef
This patch adds G_POISON as a new opcode in GlobalISel to represent poison values separately from G_IMPLICIT_DEF. This distinction is necessary to mirror LLVM IR semantics more accurately and enable better optimizations, particularly when legalization introduces padding vector elements.
---
llvm/include/llvm/Support/TargetOpcodes.def | 2 ++
1 file changed, 2 insertions(+)
diff --git a/llvm/include/llvm/Support/TargetOpcodes.def b/llvm/include/llvm/Support/TargetOpcodes.def
index 5ef3707b81fe9..b9f7c57c91c6b 100644
--- a/llvm/include/llvm/Support/TargetOpcodes.def
+++ b/llvm/include/llvm/Support/TargetOpcodes.def
@@ -297,6 +297,8 @@ HANDLE_TARGET_OPCODE(G_ABDU)
HANDLE_TARGET_OPCODE(G_IMPLICIT_DEF)
+HANDLE_TARGET_OPCODE(G_POISON)
+
/// Generic PHI instruction with types.
HANDLE_TARGET_OPCODE(G_PHI)
>From f88a6b67976b725f0a41710b843159320d3d4e06 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:36:27 +0530
Subject: [PATCH 03/10] [GlobalISel] Handle G_POISON in
isLegalOrBeforeLegalizer
This patch ensures that G_POISON is treated similarly to G_IMPLICIT_DEF in isLegalOrBeforeLegalizer.
This change is necessary to correctly distinguish poison values from undef in GlobalISel.
---
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 0dfbb91f2ac54..3b1f40e275ae9 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -328,8 +328,9 @@ bool CombinerHelper::matchCombineConcatVectors(
for (const MachineOperand &BuildVecMO : Def->uses())
Ops.push_back(BuildVecMO.getReg());
break;
- case TargetOpcode::G_IMPLICIT_DEF: {
- LLT OpType = MRI.getType(Reg);
+ case TargetOpcode::G_IMPLICIT_DEF:
+ case TargetOpcode::G_POISON: {
+ LLT OpType = MRI.getType(Reg);
// Keep one undef value for all the undef operands.
if (!Undef) {
Builder.setInsertPt(*MI.getParent(), MI);
@@ -353,9 +354,9 @@ bool CombinerHelper::matchCombineConcatVectors(
// Check if the combine is illegal
LLT DstTy = MRI.getType(MI.getOperand(0).getReg());
if (!isLegalOrBeforeLegalizer(
- {TargetOpcode::G_BUILD_VECTOR, {DstTy, MRI.getType(Ops[0])}})) {
+ {TargetOpcode::G_IMPLICIT_DEF, TargetOpcode::G_POISON, {ConcatSrcTy}}))
return false;
- }
+
if (IsUndef)
Ops.clear();
>From f0a216306359704385997d499352c72c618298ad Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:44:34 +0530
Subject: [PATCH 04/10] [GlobalISel] Add support for G_POISON in
MachineIRBuilder
This patch introduces G_POISON as a distinct opcode from G_IMPLICIT_DEF in
MachineIRBuilder, allowing better handling of poison values in GlobalISel.
---
llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 77a1a70d976d6..f903510fc860f 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -642,6 +642,11 @@ MachineInstrBuilder MachineIRBuilder::buildUndef(const DstOp &Res) {
return buildInstr(TargetOpcode::G_IMPLICIT_DEF, {Res}, {});
}
+MachineInstrBuilder MachineIRBuilder::buildPoison(const DstOp &Res) {
+ return buildInstr(TargetOpcode::G_POISON, {Res}, {});
+}
+
+
MachineInstrBuilder MachineIRBuilder::buildMergeValues(const DstOp &Res,
ArrayRef<Register> Ops) {
// Unfortunately to convert from ArrayRef<LLT> to ArrayRef<SrcOp>,
>From c0b87be9cf9a8c3d00edc5740ea80179e122a3c7 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:49:18 +0530
Subject: [PATCH 05/10] [X86][GlobalISel] Add support for G_POISON in
instruction selection
This patch updates the X86 GlobalISel instruction selector to handle G_POISON
alongside G_IMPLICIT_DEF. The changes ensure proper instruction selection and
validation for G_POISON.
- Added case for G_POISON in instruction selection.
- Updated assertions to include G_POISON.
- Ensured all relevant checks now consider G_POISON.
---
llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
index 64a1fa1780a77..c10d56b5788fc 100644
--- a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
+++ b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
@@ -428,6 +428,8 @@ bool X86InstructionSelector::select(MachineInstr &I) {
case TargetOpcode::G_BRCOND:
return selectCondBranch(I, MRI, MF);
case TargetOpcode::G_IMPLICIT_DEF:
+ case TargetOpcode::G_POISON:
+
case TargetOpcode::G_PHI:
return selectImplicitDefOrPHI(I, MRI);
case TargetOpcode::G_MUL:
@@ -1588,6 +1590,7 @@ bool X86InstructionSelector::materializeFP(MachineInstr &I,
bool X86InstructionSelector::selectImplicitDefOrPHI(
MachineInstr &I, MachineRegisterInfo &MRI) const {
assert((I.getOpcode() == TargetOpcode::G_IMPLICIT_DEF ||
+ I.getOpcode() == TargetOpcode::G_POISON ||
I.getOpcode() == TargetOpcode::G_PHI) &&
"unexpected instruction");
@@ -1604,7 +1607,8 @@ bool X86InstructionSelector::selectImplicitDefOrPHI(
}
}
- if (I.getOpcode() == TargetOpcode::G_IMPLICIT_DEF)
+ if (I.getOpcode() == TargetOpcode::G_POISON ||
+ I.getOpcode() == TargetOpcode::G_IMPLICIT_DEF)
I.setDesc(TII.get(X86::IMPLICIT_DEF));
else
I.setDesc(TII.get(X86::PHI));
>From 9a2d5b96647cc41029cbdb3f6598e4eabd59441f Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:53:17 +0530
Subject: [PATCH 06/10] [GlobalISel] Extend MIPatternMatch to support G_POISON
---
llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
index 72483fbea5805..a2a5dcb5b19f6 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
@@ -428,7 +428,9 @@ struct ImplicitDefMatch {
bool match(const MachineRegisterInfo &MRI, Register Reg) {
MachineInstr *TmpMI;
if (mi_match(Reg, MRI, m_MInstr(TmpMI)))
- return TmpMI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
+ return TmpMI->getOpcode() == TargetOpcode::G_POISON ||
+ TmpMI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
+
return false;
}
};
>From a20dc8807bec8b65a4cbcfe490dc512c89dc78b4 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 19:58:18 +0530
Subject: [PATCH 07/10] [AArch64][GISel] Add G_POISON to LegalizerInfo
---
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index fdedf44e0ba1b..49579a6184e1a 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -92,7 +92,7 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const AArch64Subtarget &ST)
const bool HasSVE = ST.hasSVE();
getActionDefinitionsBuilder(
- {G_IMPLICIT_DEF, G_FREEZE, G_CONSTANT_FOLD_BARRIER})
+ {G_IMPLICIT_DEF, G_POISON, G_FREEZE, G_CONSTANT_FOLD_BARRIER})
.legalFor({p0, s8, s16, s32, s64})
.legalFor({v16s8, v8s16, v4s32, v2s64, v2p0, v8s8, v4s16, v2s32, v4s8,
v2s16, v2s8})
>From 4a3a02fbc9141e24e9387a9fcc504e267ab22044 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 20:01:00 +0530
Subject: [PATCH 08/10] [AArch64][GISel] Extend Instruction Selection to Handle
G_POISON
Add support for G_POISON in AArch64 instruction selection, treating it similarly
to G_IMPLICIT_DEF. This ensures that poisoned values are correctly recognized
and handled during GlobalISel processing.
---
.../lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
index 67a08e39fe879..987ee9f98b62c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
@@ -3594,7 +3594,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
return selectIntrinsic(I, MRI);
case TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS:
return selectIntrinsicWithSideEffects(I, MRI);
- case TargetOpcode::G_IMPLICIT_DEF: {
+ case TargetOpcode::G_IMPLICIT_DEF:
+ case TargetOpcode::G_POISON:{
I.setDesc(TII.get(TargetOpcode::IMPLICIT_DEF));
const LLT DstTy = MRI.getType(I.getOperand(0).getReg());
const Register DstReg = I.getOperand(0).getReg();
@@ -5861,7 +5862,8 @@ bool AArch64InstructionSelector::tryOptBuildVecToSubregToReg(
if (EltRB != DstRB)
return false;
if (any_of(drop_begin(I.operands(), 2), [&MRI](const MachineOperand &Op) {
- return !getOpcodeDef(TargetOpcode::G_IMPLICIT_DEF, Op.getReg(), MRI);
+ return !getOpcodeDef(TargetOpcode::G_IMPLICIT_DEF, Op.getReg(), MRI) &&
+ !getOpcodeDef(TargetOpcode::G_POISON, Op.getReg(), MRI);
}))
return false;
unsigned SubReg;
>From 8f7c5e25a6f62a4d6194c3511874faae97416de9 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 20:02:49 +0530
Subject: [PATCH 09/10] [GlobalISel] Add G_POISON Mapping in
SelectionDAGCompat.td
Extend SelectionDAG compatibility by adding G_POISON alongside G_IMPLICIT_DEF.
This ensures correct handling of poison values in GlobalISel transformations.
---
llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td b/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
index c8c0eeb57099a..fb67e7a6b5231 100644
--- a/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
+++ b/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
@@ -58,6 +58,7 @@ def : GINodeEquiv<G_CONSTANT, imm>;
// timm must not be materialized and therefore has no GlobalISel equivalent
def : GINodeEquiv<G_FCONSTANT, fpimm>;
def : GINodeEquiv<G_IMPLICIT_DEF, undef>;
+def : GINodeEquiv<G_POISON, undef>;
def : GINodeEquiv<G_FRAME_INDEX, frameindex>;
def : GINodeEquiv<G_BLOCK_ADDR, blockaddress>;
def : GINodeEquiv<G_PTR_ADD, ptradd>;
>From 9c79e1c40550e79b61d8063467118dd3b04ae016 Mon Sep 17 00:00:00 2001
From: Devesh Yadav <160987201+Devesh-Yadav10 at users.noreply.github.com>
Date: Sat, 15 Mar 2025 20:04:16 +0530
Subject: [PATCH 10/10] [GlobalISel] Include G_POISON in LostDebugLocObserver
Extend LostDebugLocObserver to track G_POISON, ensuring consistent handling
of poison values in debug location tracking.
---
llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp b/llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp
index 6d606e5550f1a..8c19dd39a4aa8 100644
--- a/llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp
@@ -85,6 +85,7 @@ static bool irTranslatorNeverAddsLocations(unsigned Opcode) {
case TargetOpcode::G_CONSTANT:
case TargetOpcode::G_FCONSTANT:
case TargetOpcode::G_IMPLICIT_DEF:
+ case TargetOpcode::G_POISON:
case TargetOpcode::G_GLOBAL_VALUE:
return true;
}
More information about the llvm-commits
mailing list