Skip to content

Commit 18bf967

Browse files
ramntryramntry
committed
[Legalizer] Refactoring out legalizeMachineFunction
and introducing new unittests/CodeGen/GlobalISel/LegalizerTest.cpp relying on it to unit test the entire legalizer algorithm (including the top-level main loop). See also https://reviews.llvm.org/D71448
1 parent 8207c81 commit 18bf967

File tree

5 files changed

+159
-52
lines changed

5 files changed

+159
-52
lines changed

‎llvm/include/llvm/CodeGen/GlobalISel/Legalizer.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ class Legalizer : public MachineFunctionPass {
3131
public:
3232
static char ID;
3333

34-
private:
34+
struct MFResult {
35+
bool Changed;
36+
const MachineInstr *FailedOn;
37+
};
3538

39+
private:
3640
/// Initialize the field members using \p MF.
3741
void init(MachineFunction &MF);
3842

@@ -55,14 +59,19 @@ class Legalizer : public MachineFunctionPass {
5559
}
5660

5761
MachineFunctionProperties getClearedProperties() const override {
58-
return MachineFunctionProperties()
59-
.set(MachineFunctionProperties::Property::NoPHIs);
62+
return MachineFunctionProperties().set(
63+
MachineFunctionProperties::Property::NoPHIs);
6064
}
6165

6266
bool combineExtracts(MachineInstr &MI, MachineRegisterInfo &MRI,
6367
const TargetInstrInfo &TII);
6468

6569
bool runOnMachineFunction(MachineFunction &MF) override;
70+
71+
static MFResult
72+
legalizeMachineFunction(MachineFunction &MF, const LegalizerInfo &LI,
73+
ArrayRef<GISelChangeObserver *> AuxObservers,
74+
MachineIRBuilder &MIRBuilder);
6675
};
6776
} // End namespace llvm.
6877

‎llvm/lib/CodeGen/GlobalISel/Legalizer.cpp

Lines changed: 66 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -140,22 +140,13 @@ class LegalizerWorkListManager : public GISelChangeObserver {
140140
};
141141
} // namespace
142142

143-
bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
144-
// If the ISel pipeline failed, do not bother running that pass.
145-
if (MF.getProperties().hasProperty(
146-
MachineFunctionProperties::Property::FailedISel))
147-
return false;
148-
LLVM_DEBUG(dbgs() << "Legalize Machine IR for: " << MF.getName() << '\n');
149-
init(MF);
150-
const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
151-
GISelCSEAnalysisWrapper &Wrapper =
152-
getAnalysis<GISelCSEAnalysisWrapperPass>().getCSEWrapper();
153-
MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
154-
155-
const size_t NumBlocks = MF.size();
143+
Legalizer::MFResult
144+
Legalizer::legalizeMachineFunction(MachineFunction &MF, const LegalizerInfo &LI,
145+
ArrayRef<GISelChangeObserver *> AuxObservers,
146+
MachineIRBuilder &MIRBuilder) {
156147
MachineRegisterInfo &MRI = MF.getRegInfo();
157148

158-
// Populate Insts
149+
// Populate worklists.
159150
InstListTy InstList;
160151
ArtifactListTy ArtifactList;
161152
ReversePostOrderTraversal<MachineFunction *> RPOT(&MF);
@@ -178,40 +169,23 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
178169
}
179170
ArtifactList.finalize();
180171
InstList.finalize();
181-
std::unique_ptr<MachineIRBuilder> MIRBuilder;
182-
GISelCSEInfo *CSEInfo = nullptr;
183-
bool EnableCSE = EnableCSEInLegalizer.getNumOccurrences()
184-
? EnableCSEInLegalizer
185-
: TPC.isGISelCSEEnabled();
186172

187-
if (EnableCSE) {
188-
MIRBuilder = std::make_unique<CSEMIRBuilder>();
189-
CSEInfo = &Wrapper.get(TPC.getCSEConfig());
190-
MIRBuilder->setCSEInfo(CSEInfo);
191-
} else
192-
MIRBuilder = std::make_unique<MachineIRBuilder>();
193-
// This observer keeps the worklist updated.
173+
// This observer keeps the worklists updated.
194174
LegalizerWorkListManager WorkListObserver(InstList, ArtifactList);
195-
// We want both WorkListObserver as well as CSEInfo to observe all changes.
196-
// Use the wrapper observer.
175+
// We want both WorkListObserver as well as all the auxiliary observers (e.g.
176+
// CSEInfo) to observe all changes. Use the wrapper observer.
197177
GISelObserverWrapper WrapperObserver(&WorkListObserver);
198-
if (EnableCSE && CSEInfo)
199-
WrapperObserver.addObserver(CSEInfo);
178+
for (GISelChangeObserver *Observer : AuxObservers)
179+
WrapperObserver.addObserver(Observer);
180+
200181
// Now install the observer as the delegate to MF.
201182
// This will keep all the observers notified about new insertions/deletions.
202183
RAIIDelegateInstaller DelInstall(MF, &WrapperObserver);
203-
LegalizerHelper Helper(MF, WrapperObserver, *MIRBuilder.get());
204-
const LegalizerInfo &LInfo(Helper.getLegalizerInfo());
205-
LegalizationArtifactCombiner ArtCombiner(*MIRBuilder.get(), MF.getRegInfo(),
206-
LInfo);
184+
LegalizerHelper Helper(MF, LI, WrapperObserver, MIRBuilder);
185+
LegalizationArtifactCombiner ArtCombiner(MIRBuilder, MRI, LI);
207186
auto RemoveDeadInstFromLists = [&WrapperObserver](MachineInstr *DeadMI) {
208187
WrapperObserver.erasingInstr(*DeadMI);
209188
};
210-
auto stopLegalizing = [&](MachineInstr &MI) {
211-
Helper.MIRBuilder.stopObservingChanges();
212-
reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
213-
"unable to legalize instruction", MI);
214-
};
215189
bool Changed = false;
216190
SmallVector<MachineInstr *, 128> RetryList;
217191
do {
@@ -220,7 +194,8 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
220194
unsigned NumArtifacts = ArtifactList.size();
221195
while (!InstList.empty()) {
222196
MachineInstr &MI = *InstList.pop_back_val();
223-
assert(isPreISelGenericOpcode(MI.getOpcode()) && "Expecting generic opcode");
197+
assert(isPreISelGenericOpcode(MI.getOpcode()) &&
198+
"Expecting generic opcode");
224199
if (isTriviallyDead(MI, MRI)) {
225200
LLVM_DEBUG(dbgs() << MI << "Is dead; erasing.\n");
226201
MI.eraseFromParentAndMarkDBGValuesForRemoval();
@@ -240,8 +215,8 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
240215
RetryList.push_back(&MI);
241216
continue;
242217
}
243-
stopLegalizing(MI);
244-
return false;
218+
Helper.MIRBuilder.stopObservingChanges();
219+
return {Changed, &MI};
245220
}
246221
WorkListObserver.printNewInstrs();
247222
Changed |= Res == LegalizerHelper::Legalized;
@@ -254,14 +229,14 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
254229
ArtifactList.insert(RetryList.pop_back_val());
255230
} else {
256231
LLVM_DEBUG(dbgs() << "No new artifacts created, not retrying!\n");
257-
MachineInstr *MI = *RetryList.begin();
258-
stopLegalizing(*MI);
259-
return false;
232+
Helper.MIRBuilder.stopObservingChanges();
233+
return {Changed, RetryList.front()};
260234
}
261235
}
262236
while (!ArtifactList.empty()) {
263237
MachineInstr &MI = *ArtifactList.pop_back_val();
264-
assert(isPreISelGenericOpcode(MI.getOpcode()) && "Expecting generic opcode");
238+
assert(isPreISelGenericOpcode(MI.getOpcode()) &&
239+
"Expecting generic opcode");
265240
if (isTriviallyDead(MI, MRI)) {
266241
LLVM_DEBUG(dbgs() << MI << "Is dead\n");
267242
RemoveDeadInstFromLists(&MI);
@@ -291,8 +266,51 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
291266
}
292267
} while (!InstList.empty());
293268

269+
return {Changed, /*FailedOn*/ nullptr};
270+
}
271+
272+
bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
273+
// If the ISel pipeline failed, do not bother running that pass.
274+
if (MF.getProperties().hasProperty(
275+
MachineFunctionProperties::Property::FailedISel))
276+
return false;
277+
LLVM_DEBUG(dbgs() << "Legalize Machine IR for: " << MF.getName() << '\n');
278+
init(MF);
279+
const TargetPassConfig &TPC = getAnalysis<TargetPassConfig>();
280+
GISelCSEAnalysisWrapper &Wrapper =
281+
getAnalysis<GISelCSEAnalysisWrapperPass>().getCSEWrapper();
282+
MachineOptimizationRemarkEmitter MORE(MF, /*MBFI=*/nullptr);
283+
284+
const size_t NumBlocks = MF.size();
285+
286+
std::unique_ptr<MachineIRBuilder> MIRBuilder;
287+
GISelCSEInfo *CSEInfo = nullptr;
288+
bool EnableCSE = EnableCSEInLegalizer.getNumOccurrences()
289+
? EnableCSEInLegalizer
290+
: TPC.isGISelCSEEnabled();
291+
if (EnableCSE) {
292+
MIRBuilder = std::make_unique<CSEMIRBuilder>();
293+
CSEInfo = &Wrapper.get(TPC.getCSEConfig());
294+
MIRBuilder->setCSEInfo(CSEInfo);
295+
} else
296+
MIRBuilder = std::make_unique<MachineIRBuilder>();
297+
298+
SmallVector<GISelChangeObserver *, 1> AuxObservers;
299+
if (EnableCSE && CSEInfo) {
300+
// We want CSEInfo in addition to WorkListObserver to observe all changes.
301+
AuxObservers.push_back(CSEInfo);
302+
}
303+
304+
const LegalizerInfo &LI = *MF.getSubtarget().getLegalizerInfo();
305+
MFResult Result = legalizeMachineFunction(MF, LI, AuxObservers, *MIRBuilder);
306+
307+
if (Result.FailedOn) {
308+
reportGISelFailure(MF, TPC, MORE, "gisel-legalize",
309+
"unable to legalize instruction", *Result.FailedOn);
310+
return false;
311+
}
294312
// For now don't support if new blocks are inserted - we would need to fix the
295-
// outerloop for that.
313+
// outer loop for that.
296314
if (MF.size() != NumBlocks) {
297315
MachineOptimizationRemarkMissed R("gisel-legalize", "GISelFailure",
298316
MF.getFunction().getSubprogram(),
@@ -301,6 +319,5 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) {
301319
reportGISelFailure(MF, TPC, MORE, R);
302320
return false;
303321
}
304-
305-
return Changed;
322+
return Result.Changed;
306323
}

‎llvm/unittests/CodeGen/GlobalISel/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ set(LLVM_LINK_COMPONENTS
1212
add_llvm_unittest(GlobalISelTests
1313
ConstantFoldingTest.cpp
1414
CSETest.cpp
15+
LegalizerTest.cpp
1516
LegalizerHelperTest.cpp
1617
LegalizerInfoTest.cpp
1718
MachineIRBuilderTest.cpp

‎llvm/unittests/CodeGen/GlobalISel/LegalizerTest.cpp

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
//===- LegalizerTest.cpp --------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "GISelMITest.h"
10+
#include "llvm/CodeGen/GlobalISel/Legalizer.h"
11+
12+
using namespace LegalizeActions;
13+
using namespace LegalizeMutations;
14+
using namespace LegalityPredicates;
15+
16+
namespace {
17+
18+
::testing::AssertionResult isNullMIPtr(const MachineInstr *MI) {
19+
if (MI == nullptr)
20+
return ::testing::AssertionSuccess();
21+
std::string MIBuffer;
22+
raw_string_ostream MISStream(MIBuffer);
23+
MI->print(MISStream, /*IsStandalone=*/true, /*SkipOpers=*/false,
24+
/*SkipDebugLoc=*/false, /*AddNewLine=*/false);
25+
return ::testing::AssertionFailure()
26+
<< "unable to legalize instruction: " << MISStream.str();
27+
}
28+
29+
TEST_F(GISelMITest, BasicLegalizerTest) {
30+
StringRef MIRString = R"(
31+
%vptr:_(p0) = COPY $x4
32+
%v:_(<2 x s8>) = G_LOAD %vptr:_(p0) :: (load 2, align 1)
33+
$h4 = COPY %v:_(<2 x s8>)
34+
)";
35+
setUp(MIRString.rtrim(' '));
36+
if (!TM)
37+
return;
38+
39+
DefineLegalizerInfo(ALegalizer, {
40+
auto p0 = LLT::pointer(0, 64);
41+
auto v2s8 = LLT::vector(2, 8);
42+
auto v2s16 = LLT::vector(2, 16);
43+
getActionDefinitionsBuilder(G_LOAD)
44+
.legalForTypesWithMemDesc({{s16, p0, 8, 8}})
45+
.scalarize(0)
46+
.clampScalar(0, s16, s16);
47+
getActionDefinitionsBuilder(G_PTR_ADD).legalFor({{p0, s64}});
48+
getActionDefinitionsBuilder(G_CONSTANT).legalFor({s64});
49+
getActionDefinitionsBuilder(G_BUILD_VECTOR)
50+
.legalFor({{v2s16, s16}})
51+
.clampScalar(1, s16, s16);
52+
getActionDefinitionsBuilder(G_BUILD_VECTOR_TRUNC).legalFor({{v2s8, s16}});
53+
getActionDefinitionsBuilder(G_ANYEXT).legalFor({{s32, s16}});
54+
});
55+
56+
ALegalizerInfo LI(MF->getSubtarget());
57+
58+
Legalizer::MFResult Result =
59+
Legalizer::legalizeMachineFunction(*MF, LI, {}, B);
60+
61+
EXPECT_TRUE(isNullMIPtr(Result.FailedOn));
62+
EXPECT_TRUE(Result.Changed);
63+
64+
StringRef CheckString = R"(
65+
CHECK: %vptr:_(p0) = COPY $x4
66+
CHECK-NEXT: [[LOAD_0:%[0-9]+]]:_(s16) = G_LOAD %vptr:_(p0) :: (load 1)
67+
CHECK-NEXT: [[OFFSET_1:%[0-9]+]]:_(s64) = G_CONSTANT i64 1
68+
CHECK-NEXT: [[VPTR_1:%[0-9]+]]:_(p0) = G_PTR_ADD %vptr:_, [[OFFSET_1]]:_(s64)
69+
CHECK-NEXT: [[LOAD_1:%[0-9]+]]:_(s16) = G_LOAD [[VPTR_1]]:_(p0) :: (load 1)
70+
CHECK-NEXT: [[V0:%[0-9]+]]:_(s16) = COPY [[LOAD_0]]:_(s16)
71+
CHECK-NEXT: [[V1:%[0-9]+]]:_(s16) = COPY [[LOAD_1]]:_(s16)
72+
CHECK-NEXT: %v:_(<2 x s8>) = G_BUILD_VECTOR_TRUNC [[V0]]:_(s16), [[V1]]:_(s16)
73+
CHECK-NEXT: $h4 = COPY %v:_(<2 x s8>)
74+
)";
75+
76+
EXPECT_TRUE(CheckMachineFunction(*MF, CheckString)) << *MF;
77+
}
78+
79+
} // namespace

‎llvm/utils/gn/secondary/llvm/unittests/CodeGen/GlobalISel/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ unittest("GlobalISelTests") {
1616
"ConstantFoldingTest.cpp",
1717
"GISelMITest.cpp",
1818
"KnownBitsTest.cpp",
19+
"LegalizerTest.cpp",
1920
"LegalizerHelperTest.cpp",
2021
"LegalizerInfoTest.cpp",
2122
"MachineIRBuilderTest.cpp",

0 commit comments

Comments
 (0)