[llvm] 3daf7dd - [GlobalISel] Allow prelegalizer combiners to have access to LegalizerInfo.
Amara Emerson via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 2 23:53:16 PDT 2022
Author: Amara Emerson
Date: 2022-10-03T07:36:18+01:00
New Revision: 3daf7ddaef0a4ddd99d7299ab7c5882329e99700
URL: https://github.com/llvm/llvm-project/commit/3daf7ddaef0a4ddd99d7299ab7c5882329e99700
DIFF: https://github.com/llvm/llvm-project/commit/3daf7ddaef0a4ddd99d7299ab7c5882329e99700.diff
LOG: [GlobalISel] Allow prelegalizer combiners to have access to LegalizerInfo.
Before, the isPreLegalize() query in CombinerHelper only checked for the
presence of a LegalizerInfo object. This is problematic when we want to have
a combine actually check for legality in a pre-legalizer combine pass, since
if we pass a LegalizerInfo object to the constructor it causes the combines to
think that we're running *post* legalizer, which isn't true.
This change fixes it to instead check an explicit bool that passes to signal
whether the pass will be run before or after legalization.
Doing so exposed a bug in the extending loads combine, which tried to check for
legality of candidate extending loads if LegalizerInfo was present. Since we
only ran it pre-legalizer and therefore with a null LegalizerInfo, it never
actually ran. Also fixes the legality checks to keep the tests passing.
Differential Revision: https://reviews.llvm.org/D135044
Added:
Modified:
llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp
llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index ad704cb51295b..32633e2d634c2 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -113,12 +113,14 @@ class CombinerHelper {
GISelChangeObserver &Observer;
GISelKnownBits *KB;
MachineDominatorTree *MDT;
+ bool IsPreLegalize;
const LegalizerInfo *LI;
const RegisterBankInfo *RBI;
const TargetRegisterInfo *TRI;
public:
CombinerHelper(GISelChangeObserver &Observer, MachineIRBuilder &B,
+ bool IsPreLegalize,
GISelKnownBits *KB = nullptr,
MachineDominatorTree *MDT = nullptr,
const LegalizerInfo *LI = nullptr);
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 8bbecd7b3679e..204ad11d9e088 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -47,11 +47,12 @@ static cl::opt<bool>
"legal for the GlobalISel combiner"));
CombinerHelper::CombinerHelper(GISelChangeObserver &Observer,
- MachineIRBuilder &B, GISelKnownBits *KB,
- MachineDominatorTree *MDT,
+ MachineIRBuilder &B, bool IsPreLegalize,
+ GISelKnownBits *KB, MachineDominatorTree *MDT,
const LegalizerInfo *LI)
: Builder(B), MRI(Builder.getMF().getRegInfo()), Observer(Observer), KB(KB),
- MDT(MDT), LI(LI), RBI(Builder.getMF().getSubtarget().getRegBankInfo()),
+ MDT(MDT), IsPreLegalize(IsPreLegalize), LI(LI),
+ RBI(Builder.getMF().getSubtarget().getRegBankInfo()),
TRI(Builder.getMF().getSubtarget().getRegisterInfo()) {
(void)this->KB;
}
@@ -130,7 +131,7 @@ isBigEndian(const SmallDenseMap<int64_t, int64_t, 8> &MemOffset2Idx,
return BigEndian;
}
-bool CombinerHelper::isPreLegalize() const { return !LI; }
+bool CombinerHelper::isPreLegalize() const { return IsPreLegalize; }
bool CombinerHelper::isLegal(const LegalityQuery &Query) const {
assert(LI && "Must have LegalizerInfo to query isLegal!");
@@ -486,6 +487,24 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) {
return false;
}
+static unsigned getExtLoadOpcForExtend(unsigned ExtOpc) {
+ unsigned CandidateLoadOpc;
+ switch (ExtOpc) {
+ case TargetOpcode::G_ANYEXT:
+ CandidateLoadOpc = TargetOpcode::G_LOAD;
+ break;
+ case TargetOpcode::G_SEXT:
+ CandidateLoadOpc = TargetOpcode::G_SEXTLOAD;
+ break;
+ case TargetOpcode::G_ZEXT:
+ CandidateLoadOpc = TargetOpcode::G_ZEXTLOAD;
+ break;
+ default:
+ llvm_unreachable("Unexpected extend opc");
+ }
+ return CandidateLoadOpc;
+}
+
bool CombinerHelper::matchCombineExtendingLoads(MachineInstr &MI,
PreferredTuple &Preferred) {
// We match the loads and follow the uses to the extend instead of matching
@@ -538,9 +557,10 @@ bool CombinerHelper::matchCombineExtendingLoads(MachineInstr &MI,
// Check for legality.
if (LI) {
LegalityQuery::MemDesc MMDesc(MMO);
+ unsigned CandidateLoadOpc = getExtLoadOpcForExtend(UseMI.getOpcode());
LLT UseTy = MRI.getType(UseMI.getOperand(0).getReg());
LLT SrcTy = MRI.getType(LoadMI->getPointerReg());
- if (LI->getAction({LoadMI->getOpcode(), {UseTy, SrcTy}, {MMDesc}})
+ if (LI->getAction({CandidateLoadOpc, {UseTy, SrcTy}, {MMDesc}})
.Action != LegalizeActions::Legal)
continue;
}
@@ -588,12 +608,8 @@ void CombinerHelper::applyCombineExtendingLoads(MachineInstr &MI,
};
Observer.changingInstr(MI);
- MI.setDesc(
- Builder.getTII().get(Preferred.ExtendOpcode == TargetOpcode::G_SEXT
- ? TargetOpcode::G_SEXTLOAD
- : Preferred.ExtendOpcode == TargetOpcode::G_ZEXT
- ? TargetOpcode::G_ZEXTLOAD
- : TargetOpcode::G_LOAD));
+ unsigned LoadOpc = getExtLoadOpcForExtend(Preferred.ExtendOpcode);
+ MI.setDesc(Builder.getTII().get(LoadOpc));
// Rewrite all the uses to fix up the types.
auto &LoadValue = MI.getOperand(0);
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
index d655caa80ba82..3553492935d3c 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64O0PreLegalizerCombiner.cpp
@@ -73,7 +73,7 @@ class AArch64O0PreLegalizerCombinerInfo : public CombinerInfo {
bool AArch64O0PreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B, KB, MDT);
+ CombinerHelper Helper(Observer, B, /*IsPreLegalize*/ true, KB, MDT);
AArch64GenO0PreLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper);
if (Generated.tryCombineAll(Observer, MI, B))
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
index dfb531cda7e98..fbeff1370ef3f 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
@@ -364,7 +364,7 @@ bool AArch64PostLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineIRBuilder &B) const {
const auto *LI =
MI.getParent()->getParent()->getSubtarget().getLegalizerInfo();
- CombinerHelper Helper(Observer, B, KB, MDT, LI);
+ CombinerHelper Helper(Observer, B, /*IsPreLegalize*/ false, KB, MDT, LI);
AArch64GenPostLegalizerCombinerHelper Generated(GeneratedRuleCfg);
return Generated.tryCombineAll(Observer, MI, B, Helper);
}
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
index 926b8fc7c3700..d6286ba5ea59d 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
@@ -1043,7 +1043,7 @@ class AArch64PostLegalizerLoweringInfo : public CombinerInfo {
bool AArch64PostLegalizerLoweringInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B);
+ CombinerHelper Helper(Observer, B, /* IsPreLegalize*/ false);
AArch64GenPostLegalizerLoweringHelper Generated(GeneratedRuleCfg);
return Generated.tryCombineAll(Observer, MI, B, Helper);
}
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp b/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
index 50bae68b4d33a..542abd74ecdd3 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp
@@ -377,7 +377,8 @@ class AArch64PreLegalizerCombinerInfo : public CombinerInfo {
bool AArch64PreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B, KB, MDT);
+ const auto *LI = MI.getMF()->getSubtarget().getLegalizerInfo();
+ CombinerHelper Helper(Observer, B, /* IsPreLegalize*/ true, KB, MDT, LI);
AArch64GenPreLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper);
if (Generated.tryCombineAll(Observer, MI, B))
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
index 1479933a2850b..17006030089e6 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
@@ -348,7 +348,8 @@ class AMDGPUPostLegalizerCombinerInfo final : public CombinerInfo {
bool AMDGPUPostLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- AMDGPUCombinerHelper Helper(Observer, B, KB, MDT, LInfo);
+ AMDGPUCombinerHelper Helper(Observer, B, /*IsPreLegalize*/ false, KB, MDT,
+ LInfo);
AMDGPUPostLegalizerCombinerHelper PostLegalizerHelper(B, Helper);
AMDGPUGenPostLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper,
PostLegalizerHelper);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
index 98e9907068f20..6d6c69adaa658 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
@@ -198,7 +198,7 @@ class AMDGPUPreLegalizerCombinerInfo final : public CombinerInfo {
bool AMDGPUPreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- AMDGPUCombinerHelper Helper(Observer, B, KB, MDT);
+ AMDGPUCombinerHelper Helper(Observer, B, /*IsPreLegalize*/ true, KB, MDT);
AMDGPUPreLegalizerCombinerHelper PreLegalizerHelper(B, Helper);
AMDGPUGenPreLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper,
PreLegalizerHelper);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp b/llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
index bd8e568213b73..b3671eee3553b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
@@ -399,7 +399,7 @@ class AMDGPURegBankCombinerInfo final : public CombinerInfo {
bool AMDGPURegBankCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B, KB, MDT);
+ CombinerHelper Helper(Observer, B, /* IsPreLegalize*/ false, KB, MDT);
AMDGPURegBankCombinerHelper RegBankHelper(B, Helper);
AMDGPUGenRegBankCombinerHelper Generated(GeneratedRuleCfg, Helper,
RegBankHelper);
diff --git a/llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp b/llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp
index 7723a10af2d71..c16869aeb6372 100644
--- a/llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp
+++ b/llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp
@@ -61,7 +61,7 @@ bool MipsPostLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B, KB,
+ CombinerHelper Helper(Observer, B, /* IsPreLegalize*/ false, KB,
/*DominatorTree*/ nullptr, LInfo);
MipsGenPostLegalizerCombinerHelper Generated(GeneratedRuleCfg, Helper);
return Generated.tryCombineAll(Observer, MI, B, Helper);
diff --git a/llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp b/llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp
index 5dc2bf07ddd5e..237495a28f62b 100644
--- a/llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp
+++ b/llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp
@@ -38,7 +38,7 @@ class MipsPreLegalizerCombinerInfo : public CombinerInfo {
bool MipsPreLegalizerCombinerInfo::combine(GISelChangeObserver &Observer,
MachineInstr &MI,
MachineIRBuilder &B) const {
- CombinerHelper Helper(Observer, B);
+ CombinerHelper Helper(Observer, B, /*IsPreLegalize*/ true);
switch (MI.getOpcode()) {
default:
More information about the llvm-commits
mailing list